Pre-commit: framework for managing/maintaining multi-language pre-commit hooks(pre-commit.com)
pre-commit.com
Pre-commit: framework for managing/maintaining multi-language pre-commit hooks
https://pre-commit.com/
10 comments
The CI check is the important one: It's what ensures that bad formatting doesn't make it into master.
The pre-commit is just a convenience for the developer. It gives faster feedback - immediately when trying to commit, instead of a couple of minutes later. And if a developer on my team wants to disable it, it doesn't affect others.
And yes, it's important that you use the same version in dev and on CI/CD. The same applies for practically any other tool you use in your project. I see the CI/CD config as the "master" version - it's a known-working setup, that any developer should be able to use.
The pre-commit is just a convenience for the developer. It gives faster feedback - immediately when trying to commit, instead of a couple of minutes later. And if a developer on my team wants to disable it, it doesn't affect others.
And yes, it's important that you use the same version in dev and on CI/CD. The same applies for practically any other tool you use in your project. I see the CI/CD config as the "master" version - it's a known-working setup, that any developer should be able to use.
> The CI check is the important one: It's what ensures that bad formatting doesn't make it into master.
I'd say that running linters in precommit hooks is the ideal use case, as linters do not belong in CICD pipelines (i.e., it would make absolutely no sense to block a release just because a whitespace didn't matched an arbitrary rule) and linting stuff before committing also eliminates iterations in code reviews to handle inane stuff like where to break a line or how many spaces come before and after a bracket.
I'd say that running linters in precommit hooks is the ideal use case, as linters do not belong in CICD pipelines (i.e., it would make absolutely no sense to block a release just because a whitespace didn't matched an arbitrary rule) and linting stuff before committing also eliminates iterations in code reviews to handle inane stuff like where to break a line or how many spaces come before and after a bracket.
That’s a pretty reductive attitude towards linters. If writing Go (for example), linters will tell you if you have ineffective assignments, or have neglected to check an error value - both things that may well block releases.
While one could argue these are the job of the compiler, they are not today, and so linting during CI is not only appropriate, but practically required.
While one could argue these are the job of the compiler, they are not today, and so linting during CI is not only appropriate, but practically required.
> While one could argue these are the job of the compiler, they are not today, and so linting during CI is not only appropriate, but practically required.
I disagree. Stylistic choices should not block a pipeline. It's impossible to argue in favour of blocking a release just because there's a whitespace in a place where it makes some random developer frown. The robustness principle also applies here.
And no, build/critical errors are not the same as stylistic/linting errors.
I disagree. Stylistic choices should not block a pipeline. It's impossible to argue in favour of blocking a release just because there's a whitespace in a place where it makes some random developer frown. The robustness principle also applies here.
And no, build/critical errors are not the same as stylistic/linting errors.
Whether you check errors is a correctness issue, not a stylistic one.
The fact that Go requires a linter to ensure errors are properly handled is a whole other topic
I don’t disagree there.
IMO linters definitely belong into the CI/CD pipeline. I do block a release if there are whitespace issues - it's trivial for the developer to fix, and part of basic code hygiene standards that we enforce in every repository.
And if there is an emergency hotfix that needs to go out as soon as possible, we do have processes in place to bypass the CI/CD checks.
And if there is an emergency hotfix that needs to go out as soon as possible, we do have processes in place to bypass the CI/CD checks.
The no-brainer basic usage of any pre-commit hook is to make sure you don’t waste minutes while waiting for your build to finish and then end up with a failure because you don’t have a new line at the end of the file. So, adding lint(ers).
Making sure every commit have a ticket reference (and auto add it from the branch name if possible) is a fantastic convenience that also avoids commits referencing the wrong ticket.
You can also have a sanity check on what branches you don’t want to cowboy commit on unless you deliberately want to, like the main branch.
Black is a tool that is better to run on save. The key to a good pre-commit hook is that it has to be _fast_ otherwise parts of the team will stop using it.
Making sure every commit have a ticket reference (and auto add it from the branch name if possible) is a fantastic convenience that also avoids commits referencing the wrong ticket.
You can also have a sanity check on what branches you don’t want to cowboy commit on unless you deliberately want to, like the main branch.
Black is a tool that is better to run on save. The key to a good pre-commit hook is that it has to be _fast_ otherwise parts of the team will stop using it.
I have the same issue but the other way around with things like Github Actions. It's really easy to pull actions from the marketplace and apply them in your project. But now you have your linting and testing locked into Github with no easy way to do it locally. So your dev iteration cycle gets bigger. Generic linting configuration files get you a long way, but often enough there are small differences in the way test environments are setup that still causes issues.
You can use `act` to run actions locally. Most functionality is supported. Works a treat better than sending it repeatedly just to test how things behave in CI.
https://github.com/nektos/act
https://github.com/nektos/act
CICD verifies but pre commit applies.
Therefore, regardless of whether you use pre commit, your cicd should be verifying that the software, including source code is satisfying the specification.
As a collorary, pre commit doesn't need to validate. Eg tell you where exactly the errors are.
These are subtle but important distinctions as it nullifies your double accounting concern. The only doubling that exist is that the pre commit and cicd are both made aware of the specification. Maybe that is your concern.
Therefore, regardless of whether you use pre commit, your cicd should be verifying that the software, including source code is satisfying the specification.
As a collorary, pre commit doesn't need to validate. Eg tell you where exactly the errors are.
These are subtle but important distinctions as it nullifies your double accounting concern. The only doubling that exist is that the pre commit and cicd are both made aware of the specification. Maybe that is your concern.
I like seeing exactly what I commit, so I never let a pre-commit script make any changes. It's just a single command to format any changed files in the repo.
But some developers in my team do like that convenience of just being able to commit, so that's an option that can be configured in our internal tooling.
But some developers in my team do like that convenience of just being able to commit, so that's an option that can be configured in our internal tooling.
Pre-commit doesn’t commit if it makes any changes, it just leaves the changed files for you to either add to the index or re-commit skipping the checks.
Just have your CI run "pre-commit run —all-files"?
CI/CD runs from a virtual environment. I guess I have to install pre-commit framework into same virtual environment.
Granted, I haven't really looked into it too deeply, so I am a bit embarrassed with my low-effort comment above :-).
Granted, I haven't really looked into it too deeply, so I am a bit embarrassed with my low-effort comment above :-).
Here's what we do at sentry.io :) https://github.com/getsentry/sentry/blob/4cf958535563a6b7443...
That's only a problem if your CI/CD is able to have different dependency versions than a dev machine; in which case, you may end up with bigger problems too (e.g. if your Black versions are different, what about your Python interpreter versions?)
> So now I have two different black calls: in the pre-commit hook and in the CI/CD. And they must be of the same version.
What's the problem, exactly? I worked on projects which enforced precommit hooks running unit tests, and all the precommit and CICD test stages did was run the tests that were already in the repo.
Also, I don't understand what would be the problem of calling out-of-synch tests in precommit hooks and the CICD, given that precommit tests are used as a failsafe to not block the pipeline with broken commits.
What's the problem, exactly? I worked on projects which enforced precommit hooks running unit tests, and all the precommit and CICD test stages did was run the tests that were already in the repo.
Also, I don't understand what would be the problem of calling out-of-synch tests in precommit hooks and the CICD, given that precommit tests are used as a failsafe to not block the pipeline with broken commits.
The problem is if you use the recommended hooks, pulling from the tool's GitHub repo, that runs off a different venv than the rest of your project. If you leave e.g. black out of your dependencies, and only ever run it with pre-commit, there's not really an issue. I prefer to have everything managed in one place, with black as a dev dependency (I use poetry) so I never use the upstream-maintained hooks, I have to write my own.
This is why I stuff everything into a top-level Makefile with `.PHONY` rules instead. Nearly every developer knows how to invoke `make` and already has tab completion for `make` rules, to boot. The CI in turn calls `make` as well, so there's no duplication.
For example: https://github.com/trailofbits/pip-audit/blob/main/Makefile
For example: https://github.com/trailofbits/pip-audit/blob/main/Makefile
It seems like you're still assuming the hard part, ensuring that isort etc. Are installed and have the same configuration (.rc files).
Nope. The `make dev` target does that, and the configuration is checked in. You might have missed it, since most of it is embedded in the "pyproject.toml" file.
Black is an interesting example because you want two different commands in your pre-commit versus CI/CD, at least in my estimation.
In pre-commit you want to run Black in full formatter mode on the staged files. (VS Code's "Format After Save" does most of the work already, but every now and then it slips on something.) In CI/CD you want the check that formatting occurred and block it as a failed test. Those are different commands.
For what it is worth, I was evaluating lint-staged [1] and husky [2] for my pre-commit hooks.
[1] https://github.com/okonet/lint-staged
[2] https://github.com/typicode/husky
In pre-commit you want to run Black in full formatter mode on the staged files. (VS Code's "Format After Save" does most of the work already, but every now and then it slips on something.) In CI/CD you want the check that formatting occurred and block it as a failed test. Those are different commands.
For what it is worth, I was evaluating lint-staged [1] and husky [2] for my pre-commit hooks.
[1] https://github.com/okonet/lint-staged
[2] https://github.com/typicode/husky
Pre-commit supports reading in multiple types of config files, like pyproject.toml for example. It obviously depends on the tools, but in our case we can use the same configs for pre-commit and CI. It's taken some time to set up, but IMO worth it.
Curious how you used pyproject.toml. The documentation is large and searching through it didn't give me much information on how to do it, but it sounds interesting!
> So now I have two different black calls: in the pre-commit hook and in the CI/CD. And they must be of the same version.
> Can the framework run off the venv dir?
It can, but it means not using the hooks from the tool's repo and writing your own. This blog post [0] shows the pipenv version of some common hooks; mine look the same but s/pipenv/poetry.
If there's a way to use the upstream hooks with your local venv, so I can trust the tool maintainers for proper configuration, I'd love to know about it.
[0] https://sourcery.ai/blog/python-best-practices/
> Can the framework run off the venv dir?
It can, but it means not using the hooks from the tool's repo and writing your own. This blog post [0] shows the pipenv version of some common hooks; mine look the same but s/pipenv/poetry.
If there's a way to use the upstream hooks with your local venv, so I can trust the tool maintainers for proper configuration, I'd love to know about it.
[0] https://sourcery.ai/blog/python-best-practices/
My strategy is to put pre-commit in the dev requirements (which includes test-specific packages that would be irrelevant for production) and then CI can run the same hooks that pre-commit runs locally.
The objective is that ultimately no non-compliant code can be merged to master; the pre-commit hook (along with all other tests or whatever your CI enforces) is a mandatory step before a PR can be merged, and nobody is allowed to push to master directly.
The objective is that ultimately no non-compliant code can be merged to master; the pre-commit hook (along with all other tests or whatever your CI enforces) is a mandatory step before a PR can be merged, and nobody is allowed to push to master directly.
We solve this by doing `pre-commit run --all-files` in CI and if that fails, direct the author to docs on how to install pre-commit locally. Black versions are maintained/updated using renovate in the `.pre-commit-config.yaml`
https://docs.renovatebot.com/modules/manager/pre-commit/
https://docs.renovatebot.com/modules/manager/pre-commit/
Is there a reason you can't run pre-commit in CI/CD? That seems to get around your version mismatch issues.
Pre-commit has it's own CI that does this for you.
But if you're in a private repository, you can just have a job that runs pre-commit. The versions of the pre-commit tests are hard-coded into the config.
There's also a bot that keeps your pre-commit config up to date with the newest released versions of the checks.
But if you're in a private repository, you can just have a job that runs pre-commit. The versions of the pre-commit tests are hard-coded into the config.
There's also a bot that keeps your pre-commit config up to date with the newest released versions of the checks.
I use pre-commit for all of my lint. I also run lint as a test in CI/CD. In the case of github actions, it also auto-fixes the PR, if it didn't run the pre-commit, prior to commit.
Here's our setup, which is the result of several iterations and ergonomics refinements. Note: our stack is 90% python, with TS for frontend. Also 95% devs use mac (there's one data scientist on windows, he uses WSL).
We install enough utilities with `brew` to get pyenv working, use that to build all python versions. Then iirc `brew install pipx`, maybe it's `pip3 install --user pipx`. Anyway, that's the only python library binary installed outside a venv.
Pipx installs isort, black, dvc, and pre-commit.
Every repo has a Makefile. This drives all the common operations. Pyproject.toml (/eslint.json?) set the config for isort and black (or eslint). `make format` runs isort and black on python, eslint on js. `make lint` just verifies.
Pre-commit only runs the lint, it doesn't format. It also runs some scripts to ensure you aren't accidentally committing large files. Pre-commit also runs several DVC actions (the default dvc hooks) on commit, push, and checkout. These run in a venv managed by pre-commit. We just pin the version.
Github actions has a dedicated lint.yaml which runs a python linter action. We use the black version here to define which black pipx installs. We use `act` if we wanna see how an action runs without sending a commit just to trigger jobs.
As an aside, I'm still fiddling with the dvc `pre-commit` post-checkout hooks. They don't always pull the files when they ought to.
Most of the actual unit/integration tests run in containers, but they can run in a venv with the same logic, thanks to makefile. We use a dvc action to sync files in CI.
So yeah there's technically 2 copies of black and dvc, but we just use pinning. In practice, we've only had one issue with discrepancies in behavior locally vs CI, which was local black not catching a rule to avoid ''' for docstrings; using """ fixed it. On the whole, pre-commit saves against a lot of annoying goofs, but CI system is law, so we largely harmonize against that.
IMHO, this is the least egregious "double accounting" we have in local vs staging ci vs production ci (I lost that battle, manager would rather keep staing.yaml and production.yaml, rather than parameterize. Shrug.gif).
Other knowledge nuggets:
- pre-commit manages its own dependencies. This leads to surprising behavior if you aren't expecting it. Eg you need a special line to specify dvc[s3].
- black has yet to release a non-beta semver, which messes with solvers. This is super annoying. They might as well use 0ver if they don't want to commit to stability. Don't expect any kind of stability of formatting between versions. Hope they settle down soon.
- git-lfs is a nightmare. Two projects at $lastco used it. It's more trouble than it's worth. Just use DVC for yucky files. I have no affiliation with dvc, other than a few bug reports.
- makefiles are great and IMHO underrated. But they have their limits. More complex logic should be broken out into scripts.
- python dependency management is still a kafkaesque nightmare. I say this with over a decade of python experience and it's my favorite language despite this.
- Suggestions welcome!
Technologies referenced:
https://dvc.org/
https://github.com/iterative/setup-dvc
https://github.com/marketplace/actions/python-linter
https://github.com/nektos/act
We install enough utilities with `brew` to get pyenv working, use that to build all python versions. Then iirc `brew install pipx`, maybe it's `pip3 install --user pipx`. Anyway, that's the only python library binary installed outside a venv.
Pipx installs isort, black, dvc, and pre-commit.
Every repo has a Makefile. This drives all the common operations. Pyproject.toml (/eslint.json?) set the config for isort and black (or eslint). `make format` runs isort and black on python, eslint on js. `make lint` just verifies.
Pre-commit only runs the lint, it doesn't format. It also runs some scripts to ensure you aren't accidentally committing large files. Pre-commit also runs several DVC actions (the default dvc hooks) on commit, push, and checkout. These run in a venv managed by pre-commit. We just pin the version.
Github actions has a dedicated lint.yaml which runs a python linter action. We use the black version here to define which black pipx installs. We use `act` if we wanna see how an action runs without sending a commit just to trigger jobs.
As an aside, I'm still fiddling with the dvc `pre-commit` post-checkout hooks. They don't always pull the files when they ought to.
Most of the actual unit/integration tests run in containers, but they can run in a venv with the same logic, thanks to makefile. We use a dvc action to sync files in CI.
So yeah there's technically 2 copies of black and dvc, but we just use pinning. In practice, we've only had one issue with discrepancies in behavior locally vs CI, which was local black not catching a rule to avoid ''' for docstrings; using """ fixed it. On the whole, pre-commit saves against a lot of annoying goofs, but CI system is law, so we largely harmonize against that.
IMHO, this is the least egregious "double accounting" we have in local vs staging ci vs production ci (I lost that battle, manager would rather keep staing.yaml and production.yaml, rather than parameterize. Shrug.gif).
Other knowledge nuggets:
- pre-commit manages its own dependencies. This leads to surprising behavior if you aren't expecting it. Eg you need a special line to specify dvc[s3].
- black has yet to release a non-beta semver, which messes with solvers. This is super annoying. They might as well use 0ver if they don't want to commit to stability. Don't expect any kind of stability of formatting between versions. Hope they settle down soon.
- git-lfs is a nightmare. Two projects at $lastco used it. It's more trouble than it's worth. Just use DVC for yucky files. I have no affiliation with dvc, other than a few bug reports.
- makefiles are great and IMHO underrated. But they have their limits. More complex logic should be broken out into scripts.
- python dependency management is still a kafkaesque nightmare. I say this with over a decade of python experience and it's my favorite language despite this.
- Suggestions welcome!
Technologies referenced:
https://dvc.org/
https://github.com/iterative/setup-dvc
https://github.com/marketplace/actions/python-linter
https://github.com/nektos/act
Just run the same script from both places?
My favourite pre-commit feature is how it'll stash any unstaged changes before running checks when committing. No more irrelevant linter errors when committing part of a change. (And yes, it does the right thing when doing `commit -a`.)
My least favourite bug is that it doesn't always play nicely with NixOS [1], and the maintainer locked me out of the issue for pointing it out.
[1] https://github.com/pre-commit/pre-commit/pull/1938
My least favourite bug is that it doesn't always play nicely with NixOS [1], and the maintainer locked me out of the issue for pointing it out.
[1] https://github.com/pre-commit/pre-commit/pull/1938
> My least favourite bug is that it doesn't always play nicely with NixOS [1], and the maintainer locked me out of the issue for pointing it out.
Oof. Does this solve the problem https://github.com/cachix/pre-commit-hooks.nix (using Nix to manage dependencies)?
I was looking at pre-commit the other day, and wanted to incorporate it into the Nix setup of my projects.
Oof. Does this solve the problem https://github.com/cachix/pre-commit-hooks.nix (using Nix to manage dependencies)?
I was looking at pre-commit the other day, and wanted to incorporate it into the Nix setup of my projects.
Thank you for the tip. The issue is that I want the project to be usable by people who are not using Nix. IIUC I'd have to duplicate the configuration to do that with pre-commit-hooks.nix. Is that correct?
Not sure (I still haven't used it!), but I wouldn't be surprised :(
That thorn in the side bit was a little OTT.
He's doing this for free, yeah?
He's doing this for free, yeah?
Did you actually read the comment thread? My comment:
> NixOS isn't broken. I'm running into the same problem (link above).
> if you don't have a functioning ld or libstdc++ that's on you, sorry
I use it on every day basis for most of my projects. It really helps during onboarding of new team members. Before I need to expain code styles, etc. With pre-commits it simplifies whole process.
Ofc, as for every tool there is good and bad practice. For example, don't put long running tasks (eg. tests) into the pipeline. Things like code formatters, linter or import sorter is just simplify coders life.
Ofc, as for every tool there is good and bad practice. For example, don't put long running tasks (eg. tests) into the pipeline. Things like code formatters, linter or import sorter is just simplify coders life.
If you're worried about reproducible builds, make sure to use a commit ID for the `rev` property rather than a (movable) tag, optionally with a tag comment:
rev: 148ec47877499e3d671f6366f9eed812db181b40 # v1.2.0I suggest only doing pre-commit stuff for things that run really fast. Don't put your whole test suite in there.
OT: the creator has a fun live-coding and chat twitch channel worth checking out.
https://www.twitch.tv/anthonywritescode
https://www.twitch.tv/anthonywritescode
This looks very interesting, but I have a question: how to deal with multiple independent projects in the same repo? Say I have projects A, B, and C, and I made a change to a file in C. How can I make mypy run inside C only? The command would be `mypy C` from the repo root, but how can I configure pre-commit to do that?
This question also applies to GitHub Actions, which is something I'd like to implement too.
This question also applies to GitHub Actions, which is something I'd like to implement too.
You can point a hook towards a specific path, i.e. adding `files: ^C` to a hook will cause it to only run on the path of project/path C. There's also `exclude: ` to do to the opposite and other ways to customize each hook with granularity.
https://pre-commit.com/#pre-commit-configyaml---hooks
https://pre-commit.com/#pre-commit-configyaml---hooks
This works, thanks. I had to set up one mypy hook for each project, however. Is there a better way?
One obvious but often forgotten side effect of using pre-commit is that you need to be within a valid git repository with git installed.
A very nice tool, but its important to learn the details. For example, how to require multiple args in one command/one arg in multiple command.
Also, I’ve found it very helpful to always have command run from local venv, rather than remote downloads. Downloading can lead to import errors.
Overall, a very, very nice tool, but one you have to put some effort into learning.
Also, I’ve found it very helpful to always have command run from local venv, rather than remote downloads. Downloading can lead to import errors.
Overall, a very, very nice tool, but one you have to put some effort into learning.
I wrote a blog post comparing tools in this family a while back that may be of interest - https://blog.urth.org/2020/05/08/comparing-code-quality-meta...
It must run during normal CI/CD because pre-commit hooks can be skipped.
So now I have two different black calls: in the pre-commit hook and in the CI/CD. And they must be of the same version.
Ad infinitum for all other tests.
This is the reason I don't use pre-commit framework. It leads to "double accounting". I have to sync the pre-commit tests and the CI/CD tests. Or am I missing something? Can the framework run off the venv dir?