Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JSON schema and validation #182
JSON schema and validation #182
Changes from 1 commit
6e2084b
cb9dcad
3c967a7
1b70b14
27990c8
21d9d36
8aedcad
2f49788
6d33c2e
14a3b87
6b16309
27f44ec
f49acde
e02a144
1304369
cee5add
e5217aa
3ceb594
edcfd08
e135e8b
7584f53
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note here, I didn't inspect the actual schemas here deeply. I think we can adjust if they don't match what we are outputting from the application and don't need to be strict about ensuring they are strictly accurate yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we planning to add some backend tests to see if the API response conforms to the schema(s)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@agjohnson
Yes. Currently, they are "good enough" but not perfect. I've been finding issues while writing the test cases, so I expect to find more as we keep working on the addons. We already have the pattern defined, so updating them shouldn't be too hard
@zanderle
This is something that I'd eventually want but we can't prioritize right now. We will need to find a way to share the JSON schema between the two repositories and execute the tests in both platforms with the same schema. "Sharing the schema" probably means writing the schema in JSON instead of JS that's something I tried to avoid because JSON doesn't support comments among others painful things, like not supporting trailing comma 🙃
Do you have experience doing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way I did it in the past was to have the schema in json and then use
description
and$comment
keywords to provide context where needed.It's been a few years since I did this, so I can't remember how exactly I shared it between the frontend and backend repos. Let me try to find that information actually.