This was a really interesting read. I'd highly recommend it for anybody who's setting up (or currently maintains) a pre-commit workflow for their developers.
I want to add one other note: in any large organization, some developers will use tools in ways nobody can predict. This includes Git. Don't try to force any particular workflow, including mandatory or automatically-enabled hooks.
Instead, put what you want in an optional pre-push hook and also put it into an early CI/CD step for your pull request checker. You'll get the same end result but your fussiest developers will be happier.
I can second that. If there are multiple commits: https://github.com/tummychow/git-absorb is handy to add formatting changes into the right commit after commits already happened.
There's a weird thing happening on my current project. Sometimes I merge main into my branch and it fails. What fails is the pre-commit hook on the merge commit. Changes in main fail the linting checks in the pre-commit hook. But they still ended up in main, somehow. So the checks on the PR are apparently not as strict as the checks on the pre-commit hook. As a result, many developers have gotten used to committing with `--no-verify`, at which point, what is even the point of a pre-commit hook?
And sometimes I just want to commit work in progress so I can more easily backtrack my changes. These checks are better on pre-push, and definitely should be on the PR pipeline, otherwise they can and will be skipped.
Anyway, thanks for giving me some ammo to make this case.
For the sake of argument, let's say you have a check that caps the number of lines per file and that both you and main added lines in the same file. It's not too weird if that check fails only after merge, right?
One benign example of something that can break after merge even if each branch is individually passing pre-merge. In less benign cases it will your branch merged to main and actual bugs in the code.
One reason to not allow "unclean merges" and enforced incoming branches to be rebased up-to-date to be mergable to the main branch.
You probably want to run the checks on each commit to main in CI and not rely on them being consistently run by contributors.
You do you but I find rebasing my branch on main instead of merging makes me scratch mybhead way less.
Why not take the best of both worlds?
Use pre-commit hooks for client-side validation, and run the same checks in CI as well.
I’ve been using this setup for years without any issues.
One key requirement in my setup is that every hook is hermetic and idempotent. I don’t use Rust in production, so I can’t comment on it in depth, but for most other languages—from clang-format to swift-format—I always download precompiled binaries from trusted sources (for example, the team’s S3 storage). This ensures that the tools run in a controlled environment and consistently produce the same results.
> I want to add one other note: in any large organization, some developers will use tools in ways nobody can predict. This includes Git. Don't try to force any particular workflow, including mandatory or automatically-enabled hooks.
you will save your org a lot of pain if you do force it, same as when you do force a formatting style rather than letting anyone do what they please.
You can discuss to change it if some parts don't work but consistency lowers the failures, every time.
Enforcement should live in CI. Into people's dev environments, you put opt-in "enablement" that makes work easier in most cases, and gets out of the way otherwise.
It's a good thing you can't force it, because `git commit -n` exists. (And besides, management of the `.git/hooks` directory is done locally. You can always just wipe that directory of any noxious hooks.)
I can accept (but still often skip, with `git push -n`) a time-consuming pre-push hook, but a time-consuming and flaky pre-commit hook is totally unacceptable to my workflows and I will always find a way to work around it. Like everyone else is saying, if you want to enforce some rule on the codebase then do it in CI and block merges on it.
I'm the type of developer who always have a completely different way of working. I hate pre-commit hooks, and agree that pre-push + early step is CI is the right thing to do.
You shouldn't be relying on hooks to maintain the integrity of your codebase, and I'm not seeing anything here that makes me want to avoid `pre-commit` (or, more literally, the https://pre-commit.com/ tool). CI must be the source of truth for whether a commit is acceptable.
If you're using `pre-commit` the tool, not merely the hook, you can also use something like https://github.com/andrewaylett/pre-commit-action to run the tool in CI. It's a really good way to share check definitions between local development and CI, meaning you've shifted your checks to earlier in the pipeline.
I use Jujutsu day-to-day, which doesn't even support pre-commit hooks. But the tooling is still really useful, and making sure we run it in CI means that we're not relying on every developer having the hooks set up. And I have JJ aliases that help pre-commit be really useful in a JJ workflow: https://github.com/andrewaylett/dotfiles/blob/7a79cf166d1e7b...
pre-commit is a convenience for the developer to gain confidence that pre-flight checks in CI will pass. I've found trying to make them automatic just leads to pain when they interact with any non-trivial git feature and don't handle edge cases.
I've been much much happier just having a little project specific script I run when I want to do formatting/linting.
pre-commit is just a bad way to do this. 99.9% of my commits won't pass CI. I don't care. I run `git wip` which is an alias for `git commit -am "WIP"` about every 15 minutes during the day. Whenever things are in a running state. I often go back through this history on my branch to undo changes or revisit decisions, especially during refactors, especially when leveraging AI. When the most work you can lose is about 15 minutes you stop looking before you leap. Sometimes a hunch pays off and you finish a very large task in a fraction of the time you might have spent if you were ploddingly careful. Very often a hunch doesn't pay off and you have to go recover stuff from your git history, which is very easy and not hard at all once you build that muscle. The cost/benefit isn't even close, it makes me easily 2x faster when refactoring code or adding a feature to existing code, probably more. It is 'free' for greenfield work, neither helping nor really hurting. At the end the entire branch is squashed down to one commit anyway, so why would you ever not want to have free checkpoints all the time?
As I'm saying this, I'm realizing I should just wire up Emacs to call `git add {file_being_saved} && git commit -am "autocheckpoint"` every time I save a file. (I will have to figure out how to check if I'm in the middle of some operation like a merge or rebase to not mess those up.)
I'm perfectly happy to have the CI fail if I forget to run the CI locally, which is rare but does happen. In that case I lose 5 minutes or whatever because I have to go find the branch and fix the CI failure and re-push it. The flip side of that is I rarely lose hours of work, or end up painting myself in a corner because commit is too expensive and slows me down and I'm subconsciously avoiding it.
Not everyone in my team wires up their pre-commit hook to run the pre-commit tool. I use JJ, so I don't even have a pre-commit hook to wire up. But the tool is useful.
The key thing (that several folk have pointed out) is that CI runs the canonical checks. Using something like pre-commit (the tool) makes it easier to at least vaguely standardise making sure that you can run the same checks that CI will run. Having it run from the pre-commit hook fits nicely into many workflows, my own pre-JJ workflow included.
I've worked in several projects where running the tests locally automatically install pre-commit hooks and I've wanted to commit warcrimes because of it.
At my last job, we ran all of our tests, linting/formatting, etc. through pre-commit hooks. It was apparently a historical relic of a time where five developers wanted to push directly to master without having to configure CI.
To bring up jujutsu, `jj fix` (https://docs.jj-vcs.dev/latest/cli-reference/#jj-fix) is a more refined way of ensuring formatting in commits. It runs a formatting command with the diff in stdin and uses the results printed to stdout. It can simplify merges and rebases history to ensure all your commits remain formatted (so if you enable a new formatting option, it can remove the need for a special format/style fix commit in your mutable set). Hard to go back to pre-commit hooks after using jj fix (also hard to use git after using jj ;) ).
The downside currently (although I've been assured this will be fixed one day) is that it doesn't support running static analysis over each commit you want to fix.
My git rebase workflow often involves running `git rebase -x "cargo clippy -- --deny=warnings"`. This needs a full checkout to work and not just a single file input
Yeah, to add some context for people reading this, jj fix works best for edits local to the diff, and it’s meant for edits mostly. With some trickery you could run some analysis, but it’s not what jj fix is meant for right now.
What I really want is some way within jj to keep track of which commits have been checked and which are currently failing, so I can template it into log lines.
I’ve seen similar issues once hooks start doing more than fast checks. The moment they become stateful or depend on external context, they stop being guardrails and start being a source of friction. In practice, keeping them boring and deterministic seems to matter more than catching everything early.
A workflow that works well is one that takes the better ideas from Meta's "hg"+"arcanist"+edenfs+"phabricator" diff and land strategy. Git, by itself, is too low-level for shared, mostly single-source-of-truth yet distributed dev.
Make test cases all green locally before pushing, but not in a way that interferes with pushing code and they shouldn't be tied to a particular (D)VCS. Allow uploading all of the separate proposed PRs you want in a proposed "for review" state. After a PR is signed-off and sent for merging, it goes into a linearizing single source of truth backed by an automated testing/smoke testing process before they land "auto-fast-forwarded" in a mostly uncontrolled manner that doesn't allow editing the history directly. Standardization and simplicity are good, and so is requiring peer review of code before it's accepted for existing, production, big systems.
Disallow editing trunk/main/master and whenever there's merge conflict between PRs, manual rebasing of one or the other is required. Not a huge deal.
Also, have structured OWNERS files that include people and/or distribution list(s) of people who own/support stuff. Furthermore, have a USERS file that keeps lists of people who would be affected by restarting/interrupting/changing a particular codebase/service for notification purposes too. In general, monorepo and allowing submitting code for any area by anyone are roughly good approaches.
On any project where pre-commit hooks are used, the first thing I do is disable them. What I do when the code is on my side of the line isn't your business.
I agree on the other side of the fence! I quite like precommit when I use it, but I've never imposed it on any of my projects. Some people use commits sporadically then squash down- I really don't feel comfortable breaking someone's personal workflow to that degree.
I almost always have a "this cicd must pass to merge" job, that includes linting etc, and then use squash commits exclusively when merging.
Yes, big fan of enforcing standards via CI/CD workflows. Any rules a group wishes to be met should be in there. As long as someone meets those rules by the time they open a PR, I don't care how they get there.
Sure, if the warning levels are poorly tuned I might configure my LSP to ignore everything and loosen the enforcement in the build steps until I'm ready to self review. Something I can't stand with Typescript for example is when the local development server has as strict rules as the production builds. There's no good reason to completely block doing anything useful whatsoever just because of an unused variable, unreachable code, or because a test that is never going to get committed dared to have an 'any' type.
Not if I push my branch it to origin. But until I do that, it's none of your concern if I do or don't. Once it gets thrown over the wall to my colleagues and/or the general public, that's the point where I should be conforming to repo norms. Not before then.
Readit News
I want to add one other note: in any large organization, some developers will use tools in ways nobody can predict. This includes Git. Don't try to force any particular workflow, including mandatory or automatically-enabled hooks.
Instead, put what you want in an optional pre-push hook and also put it into an early CI/CD step for your pull request checker. You'll get the same end result but your fussiest developers will be happier.
And with git, you can even make anything that happens on the dev machines mandatory.
Anything you want to be mandatory needs to go into your CI. Pre-commit and pre-push hooks are just there to lower CI churn, not to guarantee anything.
(With the exception of people accidentally pushing secrets. The CI is too late for that, and a pre-push hook is a good idea.)
s/can/can't?
And sometimes I just want to commit work in progress so I can more easily backtrack my changes. These checks are better on pre-push, and definitely should be on the PR pipeline, otherwise they can and will be skipped.
Anyway, thanks for giving me some ammo to make this case.
One benign example of something that can break after merge even if each branch is individually passing pre-merge. In less benign cases it will your branch merged to main and actual bugs in the code.
One reason to not allow "unclean merges" and enforced incoming branches to be rebased up-to-date to be mergable to the main branch.
You probably want to run the checks on each commit to main in CI and not rely on them being consistently run by contributors.
You do you but I find rebasing my branch on main instead of merging makes me scratch mybhead way less.
One key requirement in my setup is that every hook is hermetic and idempotent. I don’t use Rust in production, so I can’t comment on it in depth, but for most other languages—from clang-format to swift-format—I always download precompiled binaries from trusted sources (for example, the team’s S3 storage). This ensures that the tools run in a controlled environment and consistently produce the same results.
you will save your org a lot of pain if you do force it, same as when you do force a formatting style rather than letting anyone do what they please.
You can discuss to change it if some parts don't work but consistency lowers the failures, every time.
I can accept (but still often skip, with `git push -n`) a time-consuming pre-push hook, but a time-consuming and flaky pre-commit hook is totally unacceptable to my workflows and I will always find a way to work around it. Like everyone else is saying, if you want to enforce some rule on the codebase then do it in CI and block merges on it.
Be prepared to have your PR blocked tho.
If you're using `pre-commit` the tool, not merely the hook, you can also use something like https://github.com/andrewaylett/pre-commit-action to run the tool in CI. It's a really good way to share check definitions between local development and CI, meaning you've shifted your checks to earlier in the pipeline.
I use Jujutsu day-to-day, which doesn't even support pre-commit hooks. But the tooling is still really useful, and making sure we run it in CI means that we're not relying on every developer having the hooks set up. And I have JJ aliases that help pre-commit be really useful in a JJ workflow: https://github.com/andrewaylett/dotfiles/blob/7a79cf166d1e7b...
I've been much much happier just having a little project specific script I run when I want to do formatting/linting.
As I'm saying this, I'm realizing I should just wire up Emacs to call `git add {file_being_saved} && git commit -am "autocheckpoint"` every time I save a file. (I will have to figure out how to check if I'm in the middle of some operation like a merge or rebase to not mess those up.)
I'm perfectly happy to have the CI fail if I forget to run the CI locally, which is rare but does happen. In that case I lose 5 minutes or whatever because I have to go find the branch and fix the CI failure and re-push it. The flip side of that is I rarely lose hours of work, or end up painting myself in a corner because commit is too expensive and slows me down and I'm subconsciously avoiding it.
The key thing (that several folk have pointed out) is that CI runs the canonical checks. Using something like pre-commit (the tool) makes it easier to at least vaguely standardise making sure that you can run the same checks that CI will run. Having it run from the pre-commit hook fits nicely into many workflows, my own pre-JJ workflow included.
Don't do that, just dont.
I too was about to become a war criminal.
My git rebase workflow often involves running `git rebase -x "cargo clippy -- --deny=warnings"`. This needs a full checkout to work and not just a single file input
The intended future solution is `jj run` (https://docs.jj-vcs.dev/latest/design/run/), which applies similar ideas to more general commands.
https://github.com/andrewaylett/dotfiles/blob/7a79cf166d1e7b...
What I really want is some way within jj to keep track of which commits have been checked and which are currently failing, so I can template it into log lines.
Make test cases all green locally before pushing, but not in a way that interferes with pushing code and they shouldn't be tied to a particular (D)VCS. Allow uploading all of the separate proposed PRs you want in a proposed "for review" state. After a PR is signed-off and sent for merging, it goes into a linearizing single source of truth backed by an automated testing/smoke testing process before they land "auto-fast-forwarded" in a mostly uncontrolled manner that doesn't allow editing the history directly. Standardization and simplicity are good, and so is requiring peer review of code before it's accepted for existing, production, big systems.
Disallow editing trunk/main/master and whenever there's merge conflict between PRs, manual rebasing of one or the other is required. Not a huge deal.
Also, have structured OWNERS files that include people and/or distribution list(s) of people who own/support stuff. Furthermore, have a USERS file that keeps lists of people who would be affected by restarting/interrupting/changing a particular codebase/service for notification purposes too. In general, monorepo and allowing submitting code for any area by anyone are roughly good approaches.
Lefthook with glob+stage_fixed for formatters makes one of the issues raised a complete non-issue.
I'll write a in-depth post about it maybe within the next week or so, been diving into these in my hobby projects for a year or so.
I almost always have a "this cicd must pass to merge" job, that includes linting etc, and then use squash commits exclusively when merging.
My coworker did that the other day and I'm deciding how to respond.