-
Notifications
You must be signed in to change notification settings - Fork 51
add snapcraft vendor schema and pre-commit hook #538
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 :)