search

pre-commit-ci / issues

public issues for https://pre-commit.ci
16 stars 3 forks source link

Add "ci" or "manual" to stages #105

Closed MarkusTeufelberger closed 2 years ago

MarkusTeufelberger commented 2 years ago

On one of my repositories I added a pre-commit step that takes fairly long to run (ansible-lint - https://github.com/mgit-at/ansible-collection-roles/commit/97ba01305fcaa668afa59a47e2bae05831eaa8d3) so I tagged it as [manual] since it is annoying when committing locally to wait for a few seconds/minutes until everything is done (compared to e.g. yamllint that finishes in fractions of a second).

However I don't see a way to add additional hook-ids to pre-commit.ci. Skipping seems possible, but adding some manual hooks would also be helpful. Alternatively it would be great to have a "ci" stage that only runs on ci, not locally, similar to "manual" - but that requires a change in pre-commit itself.

Possible workarounds: Turning this hook on by default is not really an option (it needs too much additional context compared to simpler linters, so it runs on the whole repository instead of just changed files which takes a while) and another workaround would be to not use pre-commit.ci at all and run pre-commit run --hook-stage manual in GitHub Actions or similar which I want to get around by using this service in the first place. I could also just not use pre-commit and just run ansible-lint in a GitHub Action, but also there I actually really like the idea of having the same environment locally and on the CI system that pre-commit offers so I don't want to do this if I don't have to.

asottile commented 2 years ago

use SKIP locally is the designed way

asottile commented 2 years ago

dupe #100

MarkusTeufelberger commented 2 years ago

How do I skip locally? I don't see it in #100?

MarkusTeufelberger commented 2 years ago

To answer my own question, apparently this feature is considered the "designed way": https://pre-commit.com/#temporarily-disabling-hooks (set an environment variable named SKIP during git commit)

This is not really a solution imho, it requires everyone(!) that checks out the repository to almost permanently have a nonstandard environment variable set just because a single repository (out of many) should not run all pre-commit hooks. Extra fun, if you have a hook with the same name in a different repo that you do want to run...

Please reconsider to add an allowlist to pre-commit.ci next to the existing skip feature.

Alternatively, please always run with --hook-stage manual enabled, since there's already the option to skip unwanted hooks anyways.

asottile commented 2 years ago

yeah I've thought a lot about this and pre-commit and pre-commit ci isn't really supposed to be used in that way. if you've got slow things they don't really belong in either. manual doesn't make sense in ci either (it's "I don't want this to run automatically").

as such I'm not going to invest in something that shouldn't be happening in the first place -- plus there's an established way to accomplish what you want via SKIP

MarkusTeufelberger commented 2 years ago

Fair enough, time to investigate how to extract the minimal necessary context for ansible-lint I guess (and how to communicate this to pre-commit) to make it fast instead of adding workarounds/features to pre-commit itself. Thanks for the quick responses and the awesome work! :-)