add snapcraft vendor schema and pre-commit hook#538
add snapcraft vendor schema and pre-commit hook#538sirosen merged 2 commits intopython-jsonschema:mainfrom
Conversation
sirosen
left a comment
There was a problem hiding this comment.
Thanks for this PR; sorry I wasn't able to get eyes on it right away!
I think we need some minor changes, which I'll apply and then I can get this merged and into a future release.
.pre-commit-hooks.yaml
Outdated
| files: > | ||
| (?x)^( | ||
| ([^/]*/)*snapcraft.yaml | ||
| )$ |
There was a problem hiding this comment.
I need to read more to see where a snapcraft.yaml may appear, but my understanding is that it can go anywhere. So I think the desired pattern here is snapcraft\.yaml with no anchors. If the current hook generator doesn't support this, I think we should extend it rather than using a less nice pattern.
There was a problem hiding this comment.
I've made a small change here, but unfortunately the way this is tested doesn't let us have what I think is the simplest pattern. The test demands a full match, but I think a suffix match would probably be best. I'll leave it only partly refined for now.
| "canonical", "snapcraft", "main", "schema/snapcraft.json" | ||
| ), | ||
| "hook_config": { | ||
| "name": "Validate snapcraft files", |
There was a problem hiding this comment.
The default description is a bit inaccurate in this case (see the lines above where it says the schema from SchemaStore).
| "name": "Validate snapcraft files", | |
| "name": "Validate snapcraft files", | |
| "description": ( | |
| "Validate snapcraft files against the schema provided by Canonical" | |
| ), |
Also fix an EOF.
|
@sirosen thank you for reviewing and merging this ❤️ |
as discussed in #535. I will be on vacation from tomorrow until beginning of April. Feel free to merge and do further commits. I will not be able to respond until then :)