Skip to content

Conversation

ilovelinux
Copy link
Contributor

Fixes #595

@ilovelinux ilovelinux force-pushed the feat/ilovelinux-595-add-yaml-files-to-gitlab-ci branch from ea76699 to 5178514 Compare September 2, 2025 09:42
@sirosen
Copy link
Member

sirosen commented Sep 3, 2025

Thanks for filing the issue and PR! I had been under the impression that GitLab only supports the .yml suffix -- that's what's in their docs, and they do not mention support for .gitlab-ci.yaml.

Can you confirm that GitLab supports the .yaml suffix and will execute pipelines from configs with this extension?

There are a couple of things which need to be tweaked for this to merge:

  • CI tests confirm that .yaml is rejected by the regex -- that needs to be updated
  • per the comment in the .pre-commit-hooks.yaml file, that hook is autogenerated, so the config which generates it needs to be updated

@andrew-rowson-lseg
Copy link

The root gitlab pipeline config can be customised per repo

Personally, I like .gitlab/.gitlab-ci.yaml. The .yml-only restriction I think only applies to pipelines that gitlab loads from external URLs.

@sirosen
Copy link
Member

sirosen commented Sep 10, 2025

Personally, I like .gitlab/.gitlab-ci.yaml.

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 check-gitlab-ci pre-commit hook to need to be parametrized extra, e.g.

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 .gitlab-ci.yaml (in the repo root), which is fine but doesn't seem like it has a strong rationale as the only exceptional case if .gitlab/* is also interesting.

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 \.gitlab-ci\.(yaml|yml) anywhere in the repo, not just the root.

@ilovelinux
Copy link
Contributor Author

As described by RFC 9512, Paragraph 2.1:

File extension(s): "yaml" (preferred) and "yml".

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 .gitlab-ci.yaml file in our repositories.

@ilovelinux
Copy link
Contributor Author

There are a couple of things which need to be tweaked for this to merge:

* CI tests confirm that `.yaml` is rejected by the regex -- that needs to be updated

Done!

* per the comment in the `.pre-commit-hooks.yaml` file, that hook is autogenerated, so the config which generates it needs to be updated

Should I revert the changes in .pre-commit-hooks.yaml?

@sirosen
Copy link
Member

sirosen commented Sep 18, 2025

Should I revert the changes in .pre-commit-hooks.yaml?

You don't have to explicitly revert, but you need to update the file where the hook is really defined (catalog.py). Otherwise, the next time the hook config gets regenerated, your change will be lost.

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 catalog.py and run make generate-hooks (or, equivalent, tox run -e generate-hooks-config), it should produce the same content that you've put into .pre-commit-hooks.yaml. So that's potentially a nice way to validate your changes.

@sirosen
Copy link
Member

sirosen commented Sep 18, 2025

Oh, I forgot to say! I'm okay with expanding to .yaml and .yml suffixes for now.

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.

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.

I came back to check in on this, and I see two things which I had missed before

  • you've already adjusted the catalog file
  • the pattern already has a leading .*, allowing for any directory (actually, probably allowing some weird stuff, like foo.github-ci.yml, but that's not new...)

So, this looks great! Sorry about the delay, I didn't realize it was ready.

@sirosen sirosen merged commit 31a8ea0 into python-jsonschema:main Sep 27, 2025
23 checks passed
@ilovelinux
Copy link
Contributor Author

Hi @sirosen, Thanks for merging! Honestly I forgot to double-check and answer you so it was a bit my fault too. 🙂

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.

GitLab CI Hook ignores .yaml files
3 participants