Discussion of the Benefits and Drawbacks of the Git Pre-Commit Hook
11 points by domenkozar
11 points by domenkozar
I have so far never seen pre-commit hooks that were bearable during every day development.
I've had significantly more success since astral arrived on the scene. I can format/lint/autofix in a pre-commit hook for a tax of ~1s per commit.
I struggled with the same issues and ended up having my team use a pre-push hook instead and adopt git-absorb and spr, which makes it easier to handle individual commits as a unit of work when your linting runs on the full branch instead of every commit.
I only use a pre-push-hook that checks whether the remote is "origin" and safeguards me against causing issues for my colleagues by running our tests.
For example for a puppet repository:
$ cat .git/hooks/pre-push
#!/bin/sh
if [ "$1" = "mars" ]; then
rake all
fi
Pre-commit hooks are awful. They always end up taking forever and then people turn them off. This means you can't trust that they actually ran which is worse than useless. CI should be the ultimate arbiter of truth in all scenarios.
Elephant in the room is that some people are migrating to jj and it sorta deprecates this commit-centric UX.
With jj I'm very rarely on top of my branch. Mostly I've switched to mid-branch to the grand-grand-grandfather to tweak it (implicit rebase is being done for me as I go). At some point, I'll want to go another two commits down. At that time it would be very clunky UX for jj to bother me with any kind of hook for either the change that I'm just leaving, or the four changes stacked on top of it. I'm thinking about something else, dammit.
As I said the whole experience is just not centered on "done up to now, check me" as I'm more free to jump around the branch.
Only pre-push hooks make sense in this context.
Are the "people" in question anything more than a small but vocal group of early-adopters? I'd be curious to know the number of jj users relative to the number of git users, but I suspect the number is quite low. I think anticipating the deprecation of a commit-centric workflow might be a tad premature.
I don't mind git pre commit hooks if we can somehow limit them only to a limited set of branches such as master, main, development.
Other than that if I make a branch locally, I should be able to do anything there. If I want to temporarily commit "hunter2" hard coded, I should be able to do that.
That's certainly possible: you can just do cur_branch=$(git rev-parse --abbrev-ref HEAD); [[ $cur_branch = master ]] || exit 0 or whatever in your pre-commit hook. I'm not sure to what extent pre-commit or husky etc lets you do this.
One question to ask here is: what specifically is accomplished by adding these checks to the commit step in the software development lifecycle, rather than anywhere else? The only real constraint I see is that secrets must be checked prior to pushing.
Examples:
pre-push hooks instead, with some effort. Why not run checks+fixes later than committing?(I personally end up doing most checks+fixes prior to pushing, to keep them out of the "inner dev loop", and to run them in parallel for each commit rather than serially.)
I would be interested to see the overlap between people who think pre-commit hooks are bad because they add friction to development, and who also believe mandatory static typing is great because it catches lots of issues quickly.
For the record: I am generally in favor of pre-commit hooks to do a few basic checks, and also generally believe that it's not that hard to have an autoformatter running on file save or a linter always-on in your editor/IDE so you don't even get to the point of trying to commit things the hooks would fail on. People who complain endlessly about pre-commit slowing them down, in my experience, stop complaining about them once those sorts of "upstream" practices get improved.
people who think pre-commit hooks are bad because they add friction to development, and who also believe mandatory static typing is great because it catches lots of issues quickly.
I would be on both of those categories, yeah.
Part of it i think comes from being old enough to remember the general SVN to git transition. SVN made network requests for basically all commands. And in comparison, git felt amazingly fast. Like, i-didn't-know-VCS-could-be-this-fast fast. Performance was the key feature of git, for me at least.
So naturally, i was surprised and pissed off when i encountered Husky —a "make pre-commit hooks mandatory" tool— on a project i had to work on. How could a freaking nodejs dependency have the galls to hijack my local git and made it abysmally slow!?
I admit that it was probably more a case of putting stuff on pre-commit that shouldn't be there. But still, it took things that used to be instant, like committing or rebasing, and made them each take several seconds instead.
But the worst part about that experience is what you already pointed out:
it's not that hard to have an autoformatter running on file save or a linter always-on in your editor/IDE so you don't even get to the point of trying to commit things the hooks would fail on
Exactly! I like having nice tools. I was already using a properly-configured editor/IDE, so, why did i need to have my local git commands slowed down to do basic things like formatting code, linting, or running the TypeScript type-checker? Were other developers pushing code upstream without even checking if it had type errors?
In the case of that gig, the answer to the latter was... maybe? Everything is possible i guess. But, i really doubt that was the main reason the pre-commit hooks were introduced. I think it was mostly a case of someone having control fantasies, wanting to mandate things on others' development setups, with a healthy mix of cargo culting: "this is the latest and hottest JavaScript trend on FAANG, we must adopt it!"
This came out rantier than i intended. I don't quite hate these tools. If someone want to use them, i'm fine with that. But i don't like making their use mandatory to others.
the galls to hijack my local git and made it abysmally slow!?
I think this is where I see the biggest disconnect. To me, having consistent standards enforced on a codebase (and pre-commit is one tool for doing that) is part of working as a team, and acknowledging that it's not my repository, it's our repository, and I have a responsibility to my colleagues to treat it with respect and make sure we all are able to work together on it. Even if it's just "my" local checkout, at any given moment I might need to share a branch with someone, so why would I want to leave it in a state that makes it harder for them to understand and work on?
And again, it's not like it's difficult to have local branches always in a state that will pass pre-commit (at least, not difficult for me; I've documented what I like to run in pre-commit).
I think it was mostly a case of someone having control fantasies, wanting to mandate things on others' development setups
I don't think this is a productive thing to project onto your colleagues.
At multiple jobs I've had to deal with tools or libraries or frameworks that I didn't particularly like. But the teams I was part of had chosen those things, and in the end it's not such a big deal to use something I don't like if it gets all of us on the same page and collaborating effectively. A standard I don't like is better than no standard at all.
Yes, we have a difference of criteria i think.
[...] acknowledging that it's not my repository, it's our repository, and I have a responsibility to my colleagues to treat it with respect and make sure we all are able to work together on it.
I don't consider the git repository on my machine as the same as the git repository that's shared (on Github or whatever). The former i consider "mine", as in, i can commit incomplete stuff, rebase & rewrite history quite freely, etc. And the latter, yes, i consider "ours", and that's where i'll have more considerations for my colleagues.
And i don't see how "respect" has anything to do with this.
The things i mentioned doing on my local repo, rebasing & rewriting history, i do them in part out of respect for my teammates. I think it's nicer to review well-factored changes, or well-written commit messages, so i put some extra effort in that. (I also like having a reasonable git history for code archeology & debugging purposes; so there's some selfish reasons for doing those things too.)
I understand that this topic is very much a trade-off, like many others. And we probably have different preferences for how it should be balanced. I personally value developer autonomy quite highly. I wouldn't see it as disrespectful if a colleague uses different tools than me. If they are happy using a different text editor, AI tools, a different git client, that's fine. As long we can collaborate effectively, i prefer them using the tools they feel more comfortable with. And, i'd like them having a similar consideration for me and my personal preferences. That's all.
When I say "different tools" I mean things like test frameworks and autoformatters.
If someone's on a Python team, for example, and the team has picked pytest and black, then everybody on the team needs to be using pytest and black, and no amount of valuing "autonomy" is going to get around that.
And even "your" local checkout, again, might need to be shared with a co-worker at pretty arbitrary moments. If you take the advice of people in this thread do the checks on pre-push rather than pre-commit, you've just slowed down collaboration because now you need to do a bunch of cleanup work to get your branch fit to push for someone else to help with. Why not just get in the habit of having your branch always be in a state where it could pass those checks? And if you're doing that, why not run them on pre-commit instead of pre-push?
The problem is contextual imo. If you have an experienced team and a mature CI/CD, pre-commit hooks are just an annoyance and they're slowing you down when experimenting. If you don't though and people write whatever code following no common team standards, nor is there proper CI; then it's one of the first "easy wins" you can get when trying to get everyone up to speed.
So there's that trade-off where you can get away with more freedom when everything is fine but you'll need to bring down the hammer, so to speak, when it's mayhem and you're trying to build a common ground everyone can build off of.
The secret to pre-commit hooks is that only near-instant, relatively important commonly hit checks should go there. The CI is for enforcing and checking stuff. Pre-commit hooks are only ever better if they give developer faster feedback. The goal of a pre-commit hook is for developer to not have to wait 5 minutes to discover that they need to make a quick fix and push again. Exactly what should go in a pre-commit hook depends on how fast it can relatively be, vs how low latency your CI has, etc. Pre-commit hooks should take less than 1s total, which is plenty of time if you don't waste it with slow tooling.
We use pre commit and pre push hooks in our monorepo. They work ok but I had to vibe up a hooks manager that allowed us to scope them by directory.