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

Question: Using hooks at different stages #768

Closed
mattlqx opened this issue Jun 15, 2018 · 4 comments
Closed

Question: Using hooks at different stages #768

mattlqx opened this issue Jun 15, 2018 · 4 comments

Comments

@mattlqx
Copy link

mattlqx commented Jun 15, 2018

  1. In order to utilize hooks that run in different stages, pre-commit needs to install itself as the hook in each one (e.g. "pre-commit" and "commit-msg"). By default however, pre-commit only installs itself in "pre-commit" and you can't specify multiple types with the -t argument. It would be nice if pre-commit would see that a hook in the config can only be used in a specific stage (e.g. "commit-msg") and install itself there rather than requiring explicit knowledge and direction from the user.
  2. Most hooks that don't explicitly limit themselves to a single stage, so if pre-commit is installed as hook multiple times (e.g. "pre-commit" and "commit-msg"), you'll get multiple runs of the unrestricted hooks for each commit. To avoid that, in your config file, you'd be required to specify a single stage for each hook. This can become pretty redundant.

For example:

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
  rev: v1.3.0
  hooks:
  # Git state
  - id: check-merge-conflict
    stages: [commit]
  - id: check-added-large-files
    stages: [commit]
  # Sensitive information
  - id: detect-private-key
    stages: [commit]
  - id: detect-aws-credentials
    stages: [commit]
  # Generic file state
  - id: trailing-whitespace
    stages: [commit]
  - id: mixed-line-ending
    stages: [commit]
  - id: end-of-file-fixer
    stages: [commit]
  - id: check-executables-have-shebangs
    stages: [commit]
  # Language syntax/formatting
  - id: check-yaml
    stages: [commit]
  - id: check-json
    stages: [commit]
  - id: pretty-format-json
    stages: [commit]
- repo: https://github.com/mattlqx/pre-commit-sign
  rev: v1.1.1
  hooks:
  - id: sign-commit

Am I understanding the limitations of stages correctly? Can anything be improved from the framework side to make commit and commit-msg stages not duplicate work or be more intelligent about what runs where? Maybe keep track of if a hook ran in the commit stage and avoid running it in the commit-msg stage?

@asottile
Copy link
Member

First: thanks for the issue! 🎉

In order to utilize hooks that run in different stages, pre-commit needs to install itself as the hook in each one (e.g. "pre-commit" and "commit-msg"). By default however, pre-commit only installs itself in "pre-commit" and you can't specify multiple types with the -t argument. It would be nice if pre-commit would see that a hook in the config can only be used in a specific stage (e.g. "commit-msg") and install itself there rather than requiring explicit knowledge and direction from the user.

The current suggestion is to just run pre-commit install more than once. Some consumers prefer to install just for push or just for commit and I don't want to break that workflow.

Am I understanding the limitations of stages correctly?

This is correct as written.

Can anything be improved from the framework side to make commit and commit-msg stages not duplicate work or be more intelligent about what runs where?

There is a bit of duplication in the configuration right now, true. I plan at some point to add something like:

default_stages: [commit]

to the top level so the duplication can be avoided (slightly) (though I'm still not 100% sold on this idea since it's nice and explicit right now).

Maybe keep track of if a hook ran in the commit stage and avoid running it in the commit-msg stage?

pre-commit currently stores zero state about runs, I don't really want to get into the business of managing that 😆

Hope this clarifies some things :)

@mattlqx
Copy link
Author

mattlqx commented Jun 15, 2018

Thanks for the quick reply.

default_stages so you can specify what the default is for hooks that aren't explicit does sound like the easiest path and resolves my concerns. I hope for it in a future release.

@asottile
Copy link
Member

yep, it's on my open source TODO

@asottile
Copy link
Member

asottile commented Jan 8, 2019

This has been released as part of v1.14.0 🎉 -- thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants