Skip to content
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

pre-commit and cookiecutter Poetry auto-generated GHA #113

Closed
ghost opened this issue Jun 23, 2022 · 5 comments
Closed

pre-commit and cookiecutter Poetry auto-generated GHA #113

ghost opened this issue Jun 23, 2022 · 5 comments
Labels
bug Something isn't working pre-commit-hooks Regarding the hooks only, not the python code

Comments

@ghost
Copy link

ghost commented Jun 23, 2022

Using Cookiecutter Poetry, I get some GHA which use an uses: ./.github/workflows/setup-poetry-env include. This is fine on GHA but pre-commit complains that:

Schema validation errors were encountered.
  .github/workflows/run-checks/action.yml::$: Additional properties are not allowed ('description', 'runs' were unexpected)
  .github/workflows/run-checks/action.yml::$: 'on' is a required property
  .github/workflows/run-checks/action.yml::$: 'jobs' is a required property
Schema validation errors were encountered.
  .github/workflows/setup-poetry-env/action.yml::$: Additional properties are not allowed ('description', 'inputs', 'runs' were unexpected)
  .github/workflows/setup-poetry-env/action.yml::$: 'on' is a required property
  .github/workflows/setup-poetry-env/action.yml::$: 'jobs' is a required property

Is there any way to exclude those files from pre-commit?

My pre-commit configuration is:

- repo: https://github.com/python-jsonschema/check-jsonschema
  rev: 0.16.1
  hooks:
  - id: check-jsonschema
    name: "Check GitHub Workflows"
    files: ^\.github/workflows/
    types: [yaml]
    args: ["--verbose", "--schemafile", "https://json.schemastore.org/github-workflow"]
@mondeja
Copy link
Contributor

mondeja commented Jun 23, 2022

See exclude

@sirosen
Copy link
Member

sirosen commented Jun 23, 2022

pre-commit excludes would definitely work. Strong 👍 to that for an immediate solution.

I would recommend trying a slightly different config:

- repo: https://github.com/python-jsonschema/check-jsonschema
  rev: 0.16.1
  hooks:
  - id: check-github-workflows
    args: ["--verbose"]

So that you use the vendored copy of the github-workflows schema. It would mean you don't get the latest schemastore schema at all times (so sometimes the vendored copy is a few commits behind schemastore), but it makes the hook behave more consistently. Changes on schemastore can't break your linting unexpectedly.


I'll take a look at the github workflows you're getting out of that project to see if maybe something is wrong with the hook config here, that it shouldn't be targeting those files. Or maybe there's something wrong with the upstream schema from schemastore. All of this when I get some more time to dig into this.

@sirosen sirosen added the needs-investigation Requires study to find an appropriate resolution label Jun 23, 2022
@ghost
Copy link
Author

ghost commented Jun 23, 2022

@mondeja

See exclude

Ah, thank you. This did the trick.

@sirosen I am not 100% sure that the cookiecutter YAML is valid. It runs fine… But that could be a side effect.

@sirosen
Copy link
Member

sirosen commented Jul 13, 2022

I filed an issue with cookiecutter-poetry to suggest a change at that end. On this end of things, I think the match rule for github-workflows validation is not correct, and that it's not correct in schemastore either.

I'm going to try to remember to do some testing to confirm that, and then I'll see about changing schemastore and the github-workflows hook.

@sirosen sirosen added bug Something isn't working pre-commit-hooks Regarding the hooks only, not the python code and removed needs-investigation Requires study to find an appropriate resolution labels Jul 14, 2022
@sirosen
Copy link
Member

sirosen commented Jul 14, 2022

I've just released v0.17.1 with a fix to the hook config that should make it no longer match on these files. If you're using exclude to avoid subdirs of the workflows dir, it should no longer be necessary.

I also have a related patch open on schemastore ( SchemaStore/schemastore#2344 ) to fix the match rule there, and the cookiecutter-poetry maintainer said he'll take up my suggested change when he's able. So I think this has surfaced a few little fixes and everything is on a good track now. 😄

Let me know if you see problems with v0.17.1!

@sirosen sirosen closed this as completed Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pre-commit-hooks Regarding the hooks only, not the python code
Projects
None yet
Development

No branches or pull requests

2 participants