-
-
Notifications
You must be signed in to change notification settings - Fork 216
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
Incorrect Revert "by default, "format" only annotates, not validates"? #660
Comments
Yeah, I agree that these tests are correct and should be added back. The vocab entry that Karen suggests in #515 is already part of the meta-schema. |
Sounds good to me too thanks for figuring this out. |
No, we don't. What you describe is true for 2020-12, but not for 2019-09, because we hadn't split the format vocabulary into the annotation and assertion varieties yet. Therefore, the problem that #515 describes is only an issue for 2019-09, and only that version's tests should have been modified. That is:
|
I have prepared a PR to fix. |
You are totally correct. My apologies. I saw the date in URIs in the commits and made a bad assumption. I should have validated that assumption. The commit was after the 2019-09 release... should have been obvious. |
PR #515 reverted/removed some tests. I believe this was in error and should not have been merged.
In the PR, @karenetheridge argues...
That's correct. They continue...
When we inspect the 2020-12 meta-schema, and the 2019-09 meta-schema, we actually find the following
$vocabulary
declarations...https://github.com/json-schema-org/json-schema-spec/blob/769daad75a9553562333a8937a187741cb708c72/schema.json#L4-L12
Git histroy shows that the line
format-annotation: true
was comitted 2020-11-24, while PR #515 was opened 2021-09-10.(I note that the linked commit was done outside of a PR and comitted directly, by me. Bad previous me. However, I did find the justification: json-schema-org/json-schema-spec#1027 (comment), so I wasn't totally rogue.)
Based on these facts, I think that PR needs to be reverted, and the tests should be included.
I found this issue when looking for these tests and not finding them.
(This Issue may be related, but I'm not 100% sure: #495)
The text was updated successfully, but these errors were encountered: