Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Pre-commit hooks are broken

jyn.dev

Post reply on HN
Print view View original

11–20 of 178 posts

Re: Pre-commit hooks are broken

#11

Post by eru »

nrclark wrote: 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…

> This includes Git. Don't try to force any particular workflow, including mandatory or automatically-enabled hooks.

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.)

Re: Pre-commit hooks are broken

#12

Post by eru »

tkzed49 wrote: post #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 deve…

Pre-commit and pre-push hooks are something developers can voluntarily add (or enable) to shorten the latency until they get feedback: instead of the CI rejecting their PR, they can (optionally!) get a local message about it.

Otherwise, I agree, your project can not rely on any checks running on the dev machine with git.

Re: Pre-commit hooks are broken

#13

Post by 000ooo000 »

thomashabets2 wrote: post #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 i…

Only a minor suggestion: git worktrees is a semi-recent addition that may be nicer than your git archive setup

Re: Pre-commit hooks are broken

#14

Post by lemonlime227 »

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 ;) ).

Re: Pre-commit hooks are broken

#15

Post by normie3000 »

eru wrote: post #11
nrclark wrote: 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…

> This includes Git. Don't try to force any particular workflow, including mandatory or automatically-enabled hooks. 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…

> with git, you can even make anything that happens on the dev machines mandatory

s/can/can't?

Re: Pre-commit hooks are broken

#16

Post by normie3000 »

nine_k wrote: post #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).

> e.g. if it contains a private key file included by mistake

Thanks - this is the first example of a pre-commit hook that I can see value in.

Re: Pre-commit hooks are broken

#17

Post by normie3000 »

thomashabets2 wrote: post #9
badgersnake wrote: post #6

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.

> I don't want to commit something that doesn't build.

This is a really interesting perspective. Personally I commit code that will fail the build multiple times per day. I only care that something builds at the point it gets merged to master.

Re: Pre-commit hooks are broken

#18

Post by dbt00 »

lemonlime227 wrote: post #14

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…

Came here to mention jj fix. It is a fundamentally more elegant way of doing things.

Re: Pre-commit hooks are broken

#19

Post by conradludgate »

lemonlime227 wrote: post #14

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…

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

Post reply on HN