Skip to content

schema for $ref keyword #211

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

Merged
merged 1 commit into from
Jan 4, 2017
Merged

Conversation

epoberezkin
Copy link
Member

@epoberezkin epoberezkin commented Dec 28, 2016

@handrews @awwright An alternative to #194, other changes seems to be merged already

Copy link
Contributor

@handrews handrews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK yeah, I can support this. I did not want to put "additionalProperties": false in with "$ref" because that really is a big change, and some schemas out there (including the draft 4 hyper-schema meta-schema!) would suddenly become invalid.

I did not want to just exclude the validation properties without a lot of discussion and thought, because excluding validation keywords but not annotation or hyper-schema keywords means that we are implying that annotation and hyper-schema keywords are valid and usable in that position, and I think defining how that works is too complicated to sneak into a PR without a larger discussion. I am still very much open to having that larger discussion, preferably for the next draft :-)

While this technically makes some schemas invalid that were valid in draft 4 (since draft 4 did not validate "$ref" syntax at all), I seriously doubt anyone was relying on such schemas. So I can support this as a good tradeoff.

@epoberezkin
Copy link
Member Author

@handrews thank you.

@epoberezkin
Copy link
Member Author

Is there an issue to discuss restrictions on $ref?

@handrews
Copy link
Contributor

handrews commented Dec 28, 2016

Is there an issue to discuss restrictions on $ref?

No. You and awwright keep ignoring my requests to open one :-) (I haven't opened it because I'm not the one who wants to change it, although I am open to doing so).

Copy link
Member

@awwright awwright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, this isn't ever going to be incorrect. Looks good.

@awwright awwright merged commit a14fb77 into json-schema-org:master Jan 4, 2017
Copy link
Member

@Relequestual Relequestual left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's merged. OK all the same.

@gregsdennis gregsdennis added clarification Items that need to be clarified in the specification and removed Type: Bug labels Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Items that need to be clarified in the specification Priority: High
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants