Live data from Hacker News

Pre-commit hooks are broken

jyn.dev

1–10 of 178 posts

Re: Pre-commit hooks are broken

#2
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.

Re: Pre-commit hooks are broken

#3
post #2

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 a…

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.

Re: Pre-commit hooks are broken

#4
Thank you. I don't need to "fix" a commit before it ends up on a remote branch. Sometimes I expect a commit to pass checks and sometimes I don't. Frankly, don't even run pre-push hooks. Just run the checks in CI when I push. You'd better be doing that anyway before I'm allowed to push to a production branch, so stop breaking my git workflows and save me the time of running duplicate checks locally.

Also, if most developers are using one editor, configure that editor to run format and auto-fix lint errors. That probably cleans up the majority of unexpected CI failures.

Re: Pre-commit hooks are broken

#5
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.

Re: Pre-commit hooks are broken

#7
A bit less enraged: pre-commit hooks should be pure functions. They must not mutate the files being committed. At best, they should generate a report. At worst, they could reject a commit (e.g. if it contains a private key file included by mistake).

Re: Pre-commit hooks are broken

#8
I feel like I found better git commands for this, that don't have these problems. It's not perfect, sure, but works for me.

The pre commit script (https://github.com/ThomasHabets/rustradio/blob/main/extra/pr...) triggers my executor which sets up the pre commit environment like so: https://github.com/ThomasHabets/rustradio/blob/main/tickbox/...

I run this on every commit. Sure, I have probably gone overboard, but it has prevented problems, and I may be too picky about not having a broken HEAD. But if you want to contribute, you don't have to run any pre commit. It'll run on every PR too.

I don't send myself PRs, so this works for me.

Of course I always welcome suggestions and critique on how to improve my workflow.

And least nothing is stateful (well, it caches build artefacts), and aside from "cargo deny" no external deps.

Re: Pre-commit hooks are broken

#9

Yep, all that and they’re also annoying. Version control tools are not supposed to argue - do what you’re told. If I messed up, the branch build will tell me.

Is that the difference between forced pre commits vs opt in? I don't want to commit something that doesn't build. If nothing else it makes future bisects annoying.

But if I intend to squash and merge, then who cares about intermediate state.

Re: Pre-commit hooks are broken

#10
Your hook can't observe a simple env var, if you are stepping off the happy path of your workflow? E.g. `GIT_HOOK_BYEBYE` = early return in hook script. Article seems a little dramatic. If you write a pre-commit hook that is a pain in your own arse, how does that make them fundamentally broken?
Post reply on HN