Skip to content

Update meta schema files in project #641

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

Closed

Conversation

christian-weiss
Copy link

Official meta schema files has been changed since they got committed to the project.
This PR is to bring them in sync with official meta schema files. This PR enables us to do easy diffs in the future.

Official:
http://json-schema.org/draft-03/schema#
http://json-schema.org/draft-04/schema#
http://json-schema.org/draft-06/schema#
http://json-schema.org/draft-07/schema#

Good news is: draft-7 and draft-6 are pretty close to official files.

Files in ./jsonschema/schema/ has been reviewed:
a) all elements are re-sorted to be in sync with element ordering of official schema files (to enable easy PR review)
b) formatting is now in sync with official schema
c) git commit history is inspected to find intended changed
d) patches of the jsonschema project are kept intact
e) rest is updated to become in sync with official meta schema files

For easyer PR review i created separate commits on each step - please review this PR commit-by-commit.

Please execute your test before merge to master to ensure that everything works as expected. You may want to check if your "fail rate" (https://github.com/ebdrup/json-schema-benchmark/blob/master/reports/jsonschema.md) has changed.
If this PR undo an intended patch (now stated in the commit history), let me know, i will re-apply it / update the PR.

BTW: i have to deal with draft-03 and draft-04 JSON instances in a project - that is the reason why i put efforts in these old meta schema files.

…al meta schema file, since it was copied to project
…roperties.id.format and properties.$schema.format was dropped to stay in sync with official meta schema, patch kept
…ted as mistakes: changed to become in sync with official draft-03
@codecov
Copy link

codecov bot commented Dec 7, 2019

Codecov Report

Merging #641 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #641   +/-   ##
======================================
  Coverage    95.5%   95.5%           
======================================
  Files          18      18           
  Lines        2471    2471           
  Branches      309     309           
======================================
  Hits         2360    2360           
  Misses         95      95           
  Partials       16      16

@Julian
Copy link
Member

Julian commented Dec 7, 2019

Thanks.

Some of this is reasonable I suppose to make merging any future changes to the metaschemas easier to merge in, but is there any other reason you had for this change?

Also there's at least one error here, not sure why you dropped the propertyNames change but that was not dropped upstream, on the contrary, we probably just haven't added it yet upstream

@christian-weiss
Copy link
Author

On the first look, i thought your files where just outdated and wanted to push the most recent version.
(I think using an outdated meta schema do not make sense)
On a second look i saw your patch. So now the PR is the official meta schem, but patched.

@christian-weiss
Copy link
Author

On draft-6:
There is properties.propertyNames, but no properties.patternProperties.propertyNames. That's the reason why properties.patternProperties.propertyNames was dropped from project file.

@Julian
Copy link
Member

Julian commented Dec 8, 2019

Again though, that change is incorrect. The upstream schemas are missing it, it should not be removed, it should be upstreamed.

@christian-weiss
Copy link
Author

I got your comment wrong and thought that you accidentally reversed properties.propertyNames & properties.patternProperties.propertyNames. Now your patch No.2 is re-applied to draft-6.

Draft-3, draft-4 and draft-7 do have the properties.patternProperties element, but your patch was missing in the old project files - i guess by intention. If not, ping me, and i will re-apply the patch No.2 to these files too.

I can not find your PR to upstream. If it is missing: now would be a good point in time to create that PR - i think (you can explain, better then me, why patch No.1 and No.2, is necessary)

@Julian
Copy link
Member

Julian commented Dec 14, 2019

Great, thanks, so what changes remain now? Is it just essentially re-ordering all the keys, and then some style differences in how to denote objects (and some trailing whitespace), or are there other functional differences you noticed?

@christian-weiss
Copy link
Author

see a1fa9cf

  • patch No1 + patch No2

@Julian
Copy link
Member

Julian commented Dec 21, 2019

OK, thanks -- I'm still a bit uncomfortable merging this given that we have not yet hit json-schema-org/JSON-Schema-Test-Suite#244 -- I'm going to have to review it carefully, which may take a bit longer.

@daniela-waranie
Copy link

daniela-waranie commented Dec 21, 2019

I my opinion you could run:

  1. test suite against current version
  2. test suite against PR version
  3. merge if both tests results are equal

That should be good enough and you should see unintended side effects.
As this represents "merged at the best current knowledge".

In addition you could leave a comment in json-schema-org/JSON-Schema-Test-Suite#244 (or in a repo-specific issue) to run the test on latest release (when improved test suite is available) and in case of test result errors to re-scan unmerged and merge version again, to check if it was introduced by the merge (revert changed file if required).

Waiting for a "perfect future" is a way to let your backlog grow more fast then it should.

@Julian
Copy link
Member

Julian commented Dec 21, 2019 via email

@christian-weiss
Copy link
Author

I did NOT spot a problem in the last 3 month. LGTM. Ready to merge. Could be released as 3.2.1-RC1 to let others send feedback, if you still worry about your test coverage.

Julian added a commit that referenced this pull request Apr 18, 2020
Julian added a commit that referenced this pull request Apr 18, 2020
@Julian Julian closed this Oct 20, 2020
@Julian Julian deleted the branch python-jsonschema:master October 20, 2020 16:37
Julian added a commit that referenced this pull request Mar 14, 2023
19947eaa1 test: unevaluatedProperties not affected by propertyNames
829270631 Check that large integers are multiples of small multipleOf
b59543f6e Merge pull request #647 from santhosh-tekuri/ref-start-slash
6e5d45d71 Merge pull request #646 from santhosh-tekuri/vocab-optional
0311dfda0 Merge pull request #651 from santhosh-tekuri/dynamicref-without-anchor
4503eeaf4 test: A $dynamicRef without anchor in fragment behaves identical to $ref
39af4c1ba test: $ref with absolute-path-reference
880c9933b test/vocabulary: ignore unrecognized optional vocabulary
fd80307ff Merge pull request #642 from santhosh-tekuri/time-2digit
a76ae650d Merge pull request #645 from json-schema-org/gregsdennis/add-vocab-tests-link
0e2b4eefd Merge pull request #643 from 0xSudarshan/main
2b78ccfc4 slight tweaking to wording
8716c4054 add link for vocab test suite to readme
c49ba5445 Fix an incorrect $schema identifier.
f0e5ce71e Added  test for schema-items alongside "ignored" additionalItems
76dae88ab Merge pull request #640 from santhosh-tekuri/refRemote-anchor
cb82e237c Merge pull request #641 from json-schema-org/gregsdennis/unevaluated-not-draft-next
e4afd233a test/format: hour, min, sec must be two digits
7efd51313 fix unevaluatedProperties/not tests for draft-next
e39c6ea6a test/refRemote: anchor within remote ref
bf51b32fb Merge pull request #639 from json-schema-org/additionalItems-unevaluatedItems
52160b368 Add a test for 2019's interaction between additional/unevaluatedItems
69a09a339 Fixed tests for annotation collection inside not.
e4e1a220b Draft7 if/then/else ref tests need to be wrapped in an allOf.
f5bd2f6c3 Merge pull request #632 from json-schema-org/ether/annotations-inside-not
626b433e5 test that annations are collected inside a "not"

git-subtree-dir: json
git-subtree-split: 19947eaa1289168a49edd21bb7a8aa2098069ae0
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