Fix issues with JSON Schema + add schema linting to CI #1476
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@sirosen I believe the CI failed because it missed the |
|
🤦 I feel silly for missing that; good catch! I've just pushed the added step as a fresh commit. I could squash it with the previous one if you have strong feelings about the git history and would like me to do so. |
|
@sirosen Don't worry! A small mistake. It'd be nice to have that commit squashed, if you don't mind. Also, adopting conventional commits for the commit messages would be great. |
This removes the "3" nested in `definitions` which makes the contents of `definitions` pass JSON Schema metaschema validation even when the contents are not valid subschemas. `definitions` SHOULD contain subschemas as its values, with no intermediate keys. (This is a MUST in later JSON Schema spec versions, in which the key switches from `definitions` to `$defs` as a related change.) The contents are intentionally *not* dedented to make review easier.
The default git diff algorithm isn't great at showing this because it incorrectly matches some closing blocks against other, distant, closing blocks. But this is all just a two-space dedent.
The `defer` definition listed `"string"` where `{"type":"string"}`
was wanted.
This is written to use `check-jsonschema` on the (current) latest version (0.27.3). It checks that the JSON Schema published in the docs is valid under its declared metaschema (`$schema`).
sirosen
force-pushed
the
jsonschema-improvements
branch
from
517045f
to
2dd909b
Compare
|
I just did a rebase to conventional-commits-ize the branch + squash those two commits. 👍 |
|
Awesome, thanks @sirosen! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #1474.
I've gone ahead and included a CI change to start running
check-jsonschemaagainst the schema in the docs dir, as mentioned in that issue, since it was part of how I validated this change.I've tried to chunk the commits for easy review:
"3")deferdefinition in JSON Schema ("string" -> {"type": "string"}).github/workflows/lint.yaml)Aside:
I've been a little bit biased in check-jsonschema towards developers with workflows like my own and haven't thought hard enough about use-cases like this repo. I'll be thinking harder about providing docker containers to run the tool, perhaps some experimental binary builds (there are tools for this like pyinstaller) and maybe a GitHub Action too.
One thing which GitHub Actions has, which I would really like to be able to offer, is the integration with Dependabot to help you stay up to date as the tool evolves. So that's another angle on this.
I can't promise anything yet, but I'd like to make installing and using it easier for non-Python projects.