confine pre-commit to stages

[davidculley]

See https://pre-commit.com/#confining-hooks-to-run-at-certain-stages

If you are authoring a tool, it is usually a good idea to provide an appropriate stages property. For example a reasonable setting for a linter or code formatter would be stages: [pre-commit, pre-merge-commit, pre-push, manual].

[staticdev]

@davidculley thanks for contributing. Could you elaborate on the changes it will bring to current behavior of pre-commit and what are pros and cons of having this? What are the default stages?

[davidculley]

Sure.

pre-commit supports many types of git hooks. See:

https://pre-commit.com/#supported-git-hooks

Of all these possible types, only the type pre-commit is used/installed by default. That is, after pip install pre-commit && pre-commit install, you only have the type pre-commit installed. Optionally, you can install additional types.

For example, you could additionally install the hook type commit-msg to make pre-commit ensure that commit messages follow the convention conventional commits, or to prevent duplicate Signed-off-by trailers. You'd install that optional additional hook type with the command pre-commit install --hook-type commit-msg.

Without the changes in this pull request, pre-commit would run isort at every stage that is installed. But not every stage (hook type) makes sense for running isort. isort should run only at a sensible subset of all the available hook types.

With the changes in this pull request, isort would no longer run, for example, after amending a commit or after a rebase, even if a user had installed the hook type post-rewrite using pre-commit install --hook-type post-rewrite because, according to the pre-commit documentation, it doesn't make sense to run isort at that stage.

Providers of hooks can select which git hooks they run on by setting the stages property in .pre-commit-hooks.yaml.

a reasonable setting for a linter or code formatter would be stages: [pre-commit, pre-merge-commit, pre-push, manual].

If stages is not set […] the default value will be pulled from the top-level default_stages option (which defaults to all stages). By default, tools are enabled for every hook type that pre-commit supports.

I don't know if there are cons to not having this, but the developer of pre-commit recommends adding it:

If you are authoring a tool, it is usually a good idea to provide an appropriate stages property.

One point to remember is:

In mid/late 2024, we'd have to require at least pre-commit version 3.2.0 and change

minimum_pre_commit_version: '2.9.2'

to

minimum_pre_commit_version: '3.2.0'

Reason: Probably around that time, pre-commit will be released in version 4.x.x and version 4 will require

stages: [pre-commit, pre-merge-commit, pre-push, manual]

(notice the prefix pre-) instead of the current proposal

stages: [commit, merge-commit, push, manual]

but the new version 4 syntax will not work with versions of pre-commit older than 3.2.0, so some users still stuck on a version older than 3.2.0 would complain if we added the newer syntax right away today.

[staticdev]

@davidculley thanks a lot for the explanation! It is clear now, lgtm.

[staticdev]

Thanks again.

[davidculley]

You're welcome. I'm happy I could be useful.

[staticdev]

Btw @davidculley your change is already in last release