Skip to content

Replace '$defs' with 'definitions' in draft 6,7 #582

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
Aug 6, 2022
Merged

Replace '$defs' with 'definitions' in draft 6,7 #582

merged 1 commit into from
Aug 6, 2022

Conversation

santhosh-tekuri
Copy link
Contributor

@santhosh-tekuri santhosh-tekuri commented Aug 6, 2022

in draft 6 and 7, anchors are not looked up in $defs

@santhosh-tekuri santhosh-tekuri requested a review from a team as a code owner August 6, 2022 16:33
@Julian
Copy link
Member

Julian commented Aug 6, 2022

Thanks! There's two more of these in id.json. Mind fixing there too? Appreciated.

@santhosh-tekuri
Copy link
Contributor Author

i was testing my validator https://github.com/santhosh-tekuri/jsonschema with latest changes, and noticed that those two tests were failings.

other tests having $defs were passing, because they were using explicit json pointer

if you say yes, i will update the rest of them also.

@Julian
Copy link
Member

Julian commented Aug 6, 2022

Ah indeed, ok, they're not strictly wrong, but still a bit stylistically retcon'ed, but fair enough, will merge these as is to fix the bug and we can change those after. Thanks for the fix!

@Julian Julian merged commit c4341bd into json-schema-org:main Aug 6, 2022
@handrews
Copy link
Contributor

handrews commented Aug 6, 2022

@Julian starting with draft-07 there is wording around whether the reference target is a schema. In 2019-09 the spec explicitly notes that referencing something that cannot be determined to be a schema has behavior that is either undefined or implementation-specific, depending on the exact circumstances.

In draft-07, the wording is a bit more ambiguous:

If the resulting URI identifies a schema within the current document,
or within another schema document that has been made available to the
implementation, then that schema SHOULD be used automatically.

But it definitely implies that a referencing URI might not identify a schema, and therefore might not be dereferenced.

So this is more than a stylistic concern, at least for the draft-07 tests. Draft-06's language still implied that the reference target would be treated as a schema, so the exact keyword there is arguably stylistic. But from draft-07 on that is not the case and only definitions (for draft-07) or $defs (for later) is a truly valid test.

@handrews
Copy link
Contributor

handrews commented Aug 6, 2022

In 2019-09 and later, definitions is reserved for compatibility, but implementations are not required to support it, so mandatory tests would need to use $defs and optional compatibility tests could be added for definitions.

@Julian
Copy link
Member

Julian commented Aug 6, 2022

Are those comments meant for this PR? Or are you talking about what we were talking about at the end, i.e. #583?

Regardless, sounds like you're agreeing with the merges?

@handrews
Copy link
Contributor

handrews commented Aug 7, 2022

@Julian two PRs with the same title? er... I'm confused about something... but I was responding to:

Ah indeed, ok, they're not strictly wrong, but still a bit stylistically retcon'ed

Which I interpreted (or misinterpreted) to be a comment that it's not strictly wrong to use $defs with draft-06 and draft-07. That's correct for draft-06, but arguably not for draft-07. But it's kinda debatable even for draft-07 so perhaps I shouldn't have commented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants