-
-
Notifications
You must be signed in to change notification settings - Fork 215
adding specification enhancement for additionalProperties 2020-12 #726
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
Changes from 1 commit
50a2028
003ac02
340116e
1362a8c
9b169be
51fc69c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,16 @@ | |
"type": "array", | ||
"items": { "$ref": "#/$defs/test" }, | ||
"minItems": 1 | ||
}, | ||
"specification":{ | ||
"description": "Location of the test case in the specification", | ||
"type": "object", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect an array is more correct here actually , which would allow referencing multiple locations, perhaps even within one spec. (Or if we want to get fancy at the expense of making it harder to parse we could do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Concluding this, which of them is better to be chosen:
I felt first one is simpler, but doesn't mention its value is a "section" explicitely There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Always having an array is easier to handle in some languages. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'd go with number 1 personally.
Makes sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. great |
||
"minProperties": 1, | ||
"properties":{ | ||
"jsonschema": {"type": "string"}, | ||
"rfc": {"type": "string"} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these both URIs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They are URL's for a particular section in the JSON Schema Spec. They will redirect to the location where test-case is more relevant. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the structure of "specification" property supposed to be something else There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2.. Actually each section of the draft has many paragraphs, and each paragraph has unique URI's.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If you look at this format it has a common pattern.
I think it'd be better just to include the last part of that, the section and paragraph. If desired, tooling can construct the URL. The spec name ( Again, this is just a reference for documentation purposes. It doesn't need to be functional. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I totally got that. We can have spec name: core/validation and for rfc: rfc3456/rfc4567 or ... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Julian what is the philosophy of tests for requirements like "behaves per X spec, section Y"? Do we include tests checking behavior against other specs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like why would we include that info? Yeah we definitely have lots of that, see e.g. all the format tests, all of which essentially are testing behavior specified in some other specification and generally RFC. But def fine to leave it out to start I think. (I haven't read the rest of the thread yet!) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the example structure for a test-case be:
and because there may be more than one place in spec, we can make for example:
can keep it such that And instead of asking can I make as many commits as possible and then take comments? |
||
}, | ||
"additionalProperties": false | ||
} | ||
}, | ||
"additionalProperties": false | ||
|
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.
It's not just "the" specification, given it can either be the JSON Schema specification or some other specification. Probably we should elaborate here with some guidance for how to use this, specifically:
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.
Got this 👍