Skip to content

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

Merged
merged 2 commits into from
Mar 26, 2025

Conversation

fabolhak
Copy link
Contributor

@fabolhak fabolhak commented Mar 11, 2025

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 :)

Copy link
Member

@sirosen sirosen left a 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.

files: >
(?x)^(
([^/]*/)*snapcraft.yaml
)$
Copy link
Member

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.

Copy link
Member

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",
Copy link
Member

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).

Suggested change
"name": "Validate snapcraft files",
"name": "Validate snapcraft files",
"description": (
"Validate snapcraft files against the schema provided by Canonical"
),

@sirosen sirosen merged commit 25be81a into python-jsonschema:main Mar 26, 2025
1 check passed
@fabolhak
Copy link
Contributor Author

fabolhak commented Apr 8, 2025

@sirosen thank you for reviewing and merging this ❤️

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.

2 participants