-
-
Notifications
You must be signed in to change notification settings - Fork 240
Add specification links for type keyword tests #807
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
base: main
Are you sure you want to change the base?
Add specification links for type keyword tests #807
Conversation
tests/draft2020-12/type.json
Outdated
| { | ||
| "vocabulary": "validation", | ||
| "url": "https://json-schema.org/draft/2020-12/json-schema-validation#type", | ||
| "quote": "The value of this keyword MUST be either a string or an array. If it is an array, elements of the array MUST be strings and MUST be unique." |
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.
This isn't a good quote, it doesn't relate to the test being written in any of these cases. I'd either quote the relevant paragraph which is below it, or since it's such basic functionality, less fragile is just to use a reference to the section heading without a quote.
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.
"quote": "See JSON Schema Validation §6.1.1 Type"
should i add quote this way ?
@Julian
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.
No, have a look at the (underdocumented) https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/main/test-schema.json#L48 and feel free to suggest a doc improvement for someone working on this, but you should essentially use validation: <section number> for the entirety of this object if my recollection is right. You can look around for other examples in the suite as well most likely, it'll look something like that.
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.
@Julian
I have added the validation: section number and removed the quote.
|
@Shristibot is this related to an issue? We need to open issues and discuss them before opening PRs. I'm not convinced this is necessary. |
@gregsdennis |
|
No, @Shristibot , this (#807, what you linked to) is a PR. We need an issue where this proposed change can be discussed before a change is decided on and made. |
You’re right — I mistakenly linked a PR. The relevant issue is: #699 |
80b369b to
890ea1f
Compare
890ea1f to
6d699e7
Compare
Added specification field to the type tests in type.json so each test now points to the official JSON Schema type keyword docs: https://json-schema.org/draft/2020-12/json-schema-validation#type
This helps anyone reading the tests understand which part of the spec each test is based on. No changes to test logic, just added reference links.