search

python-jsonschema / check-jsonschema

A CLI and set of pre-commit hooks for jsonschema validation with built-in support for GitHub Workflows, Renovate, Azure Pipelines, and more!
https://check-jsonschema.readthedocs.io/en/stable
Other
207 stars 40 forks source link

feat: add mkdocs-material schema to catalog #324

Closed skwde closed 8 months ago

skwde commented 1 year ago

Schema json for MkDocs Material (https://github.com/squidfunk/mkdocs-material)

See https://squidfunk.github.io/mkdocs-material/creating-your-site/ for filename and schema link.

sirosen commented 1 year ago

:+1: looks good!

I'll need to run some jobs to generate configs from this, so I'll circle back to do that and merge (unless I run into any issues) later today or this week at the latest.

sirosen commented 1 year ago

Okay, unfortunately, there's a bit of a problem with this addition, so it won't be a trivial merge (as some of the other schemas have been).

I took this branch and trialed merging to main and fixing tests with tox r -e generate-hooks. (which worked) I then added a couple of positive and negative test cases for the new schema and ran the testsuite, which failed, unexpectedly. I've put the result in a branch on this repo ( https://github.com/python-jsonschema/check-jsonschema/tree/add-mkdocs-material ) by rebasing this PR branch and making the changes in a subsequent commit. Please feel free to pull it back into this PR if you like -- or I can continue to tinker with that branch of work.

It turns out, the mkdocs schema is using relative references to other schemas from their site. It's completely fine for fully-online JSON Schema usage, but it breaks vendoring in significant ways. It can be band-aided with --base-url pretty quickly, but that won't be sufficient for all users. In particular, pre-commit.ci runs builds without any network access, so this needs some more thought.

I'm going to need to reach out the mkdocs maintainers and see what kind of resolution seems appropriate. We can vendor all referenced schemas today and try to work that way, but I'd like to check with them before taking such a drastic measure.

skwde commented 1 year ago

Ohh, that is unfortunate.

I don't really mind which branch we continue to use. In case I can help somehow. I can also work on your new branch.

sirosen commented 1 year ago

Just FYI, this problem is, it turns out, worse than I thought. Not only is there a tree of schemas to vendor, but their schemas refer out to 3rd party schemas.

I opened a discussion to ask for the mkdocs-material maintainers' advice and input. The result is pushing me to reconsider trying to do this. The fact that we have to account for those 3rd party schemas somehow, and that they might not even be a complete definition of the file's contents, makes me pretty nervous.

I'm going to keep chewing on this, and I'll get involved if mkdocs-material is open to producing an inlined variant of their schema. But for the moment I'm sort of putting this feature on the shelf -- too many open questions and difficult problems.

sirosen commented 8 months ago

I'm closing this, as I don't think it's likely to happen without some changes on the mkdocs-material side, which I'm not sure they want to or even should make. The discussion linked above has a lot more details for anyone curious. Basically, we want different things out of the single schema and I don't see a way to make their schema (which IMO is theirs to maintain as they see fit) meet the requirements of a pre-commit hook.

I greatly appreciate the effort and initiative shown in opening this PR! But closing is the best/clearest outcome in this case.