-
Notifications
You must be signed in to change notification settings - Fork 56
Add support for .gitlab-ci.yaml
files
#596
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
base: main
Are you sure you want to change the base?
Add support for .gitlab-ci.yaml
files
#596
Conversation
ea76699
to
5178514
Compare
Thanks for filing the issue and PR! I had been under the impression that GitLab only supports the Can you confirm that GitLab supports the There are a couple of things which need to be tweaked for this to merge:
|
The root gitlab pipeline config can be customised per repo Personally, I like |
Yeah, that makes total sense to me. If I used GitLab more actively, I could see adopting something similar. If you customize the path at which the file is found, my initial thought is that it makes sense for the repos:
- repo: https://github.com/python-jsonschema/check-jsonschema
rev: 0.33.0
hooks:
# match any YAML files in .gitlab/ ending in '-ci'
- id: check-gitlab-ci
files: ^\.gitlab/.*-ci\.(yaml|yml)$ This PR is changing the default rule to match on I looked at schemastore, and they have some very permissive globs for gitlab-ci. Perhaps the correct solution is to follow their rule, which is to accept |
As described by RFC 9512, Paragraph 2.1:
As @andrew-rowson-lseg pointed out, it's not the default behavior but, since our company is following the suggestions of the RFC, we are actually using |
Done!
Should I revert the changes in |
You don't have to explicitly revert, but you need to update the file where the hook is really defined ( I've documented this very briefly here in the contrib doc. A few people have been confused by it, so I think I need to update it to be a bit clearer in terms of signposting. If you update |
Oh, I forgot to say! I'm okay with expanding to However, I think it's best to be even more permissive and follow the schemastore matching rule: https://github.com/SchemaStore/schemastore/blob/98eeb8ccb7cbf6617c848136a0041a197fd72678/src/api/json/catalog.json#L2712-L2717 I can do that as a fast-follow after this work, or if you would rather expand the match to follow the above rule, I'd be happy for that to be included in this PR. |
Fixes #595