-
-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add tests for references and change in rules for draft3 to 4 #54
Conversation
"$ref": "#/disallow/1" | ||
}, | ||
"to": { | ||
"allOf": [ |
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 is valid, but shouldn't the subschemas in allOf
be in the opposite order? i.e. the disallow
keyword first disallows string
, so shouldn't the type string not
be first in the array?
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.
Yeah! by doing it this way I can get the indexes of objects, like
If I choose to upgrade index 0: string
... Then I will lose track of the index where the object was present.
If string
is popped first, then array will be [{}]
, then I lose track that it was in index 1, because now its in index 0
"allOf": [ | ||
{} | ||
], | ||
"$ref": "#/allOf/1" |
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 is not valid, as allOf
only has one element?
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.
@jviotti in the draft4: https://json-schema.org/draft-04/schema It has constaints minItems: 1, so it can have 1 item I think but not 0 items...
I left a few minor comments, but it's shaping up great! |
@jviotti adding tests for references too