WIP converting all static strings over to draft 2020-12 #343
Conversation
| schema.erase(attr); | ||
|
|
||
| // special case where we break draft-7 and allow overriding of properties when a $ref is used | ||
| // special case where we break draft-7 and allow overriding of properties when a $ref is used <<-- might be out-of-date now that 2020-12 is used |
There was a problem hiding this comment.
Can someone help me understand the meaning in this comment?
| // A class representing a JSON-URI for schemas derived from | ||
| // section 8 of JSON Schema: A Media Type for Describing JSON Documents | ||
| // draft-wright-json-schema-00 | ||
| // draft-wright-json-schema-00 <<-- might be incorrect now that 2020-12 is used |
src/nlohmann/json-schema.hpp
Outdated
| { | ||
|
|
||
| extern json draft7_schema_builtin; | ||
| extern json draft_2020_12_schema_builtin; |
There was a problem hiding this comment.
I left this general approach here, because I suspect that the intent is to allow for multiple versions of the schema to be included simultaneously -- and validate against the one indicated by $schema. Is that correct?
If not, then this variable should be renamed to something more generic, like schema_builtin.
| "description": "valid definition schema", | ||
| "data": { | ||
| "definitions": { | ||
| "$defs": { |
There was a problem hiding this comment.
I don't think this is correct; the test is still failing (er, passing when it should fail). Could someone help me understand the structure here better?
There was a problem hiding this comment.
I have barely stuck my toe into json schema definitions, so it is entirely possible that your question is mostly over my head. But draft 2020-12 allows either definitions or $defs as a keyword. Is that what you were expecting to fail? Because if so, it should, in fact, pass. My PR #338 allows either to pass precisely for this reason.
There was a problem hiding this comment.
I think I wrote that comment when I was still trying to get my bearings on stuff.
I think you'll agree that $defs is "correct", which definitions is the backward compatible safety net, no?
At least, according to https://json-schema.org/draft/2020-12/schema
There was a problem hiding this comment.
I think you'll agree that $defs is "correct", which definitions is the backward compatible safety net, no?
That's the way I read it as well.
| "description": "invalid definition schema", | ||
| "data": { | ||
| "definitions": { | ||
| "$defs": { |
|
@JohanMabille any comments or helpful hints on this incomplete effort would be appreciated |
|
Sorry that I didn't reply earlier, but actually I was a little surprised that this PR is for real. Sorry to be that harsh. Draft 2020-12 contains a lot of changes other than the Adding submodules for the schema is not really a good idea either. The schema is a standardized thing, it won't change for one version of a draft. I'd rather have a copy of a text-file committed, than carrying submodules of third-party projects with us. This PR, as it is stands of today, won't be merged. 😞 That said, your initiative to support 2020-12 is very welcome. 😄 This would have to be discussed beforehand. And probably cut in smaller tasks to be done by several people. |
3 participants
No description provided.