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

81–90 of 178 posts

Re: Pre-commit hooks are broken

#81

Post by andrewaylett »

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

Re: Pre-commit hooks are broken

#82

Post by andrewaylett »

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…

Case in point: https://www.jj-vcs.dev/

Re: Pre-commit hooks are broken

#83

Post by Spivak »

andrewaylett wrote: post #81

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…

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.

Re: Pre-commit hooks are broken

#84

Post by andrewaylett »

oxryly1 wrote: post #71
Mic92 wrote: post #3

Earlier quoted context omitted.

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.

It looks like git absorb rewrites history. Doesn’t that break your previously pushed branch?

That's a controversy I'm not sure you necessarily realise you've stepped into :).

It's fairly common to consider working and PR branches to be "unpublished" from a mutability point of view: if I base my work on someone else's PR, I'm going to have to rebase when they rebase. Merging to `main` publishes the commit, at which point it's immutable.

Working with JJ, its default behaviour is to consider parents of a branch that's not owned by you to be immutable.

Re: Pre-commit hooks are broken

#85

Post by ltbarcly3 »

Spivak wrote: post #83
andrewaylett wrote: post #81

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…

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.

Re: Pre-commit hooks are broken

#86

Post by nirvdrum »

ltbarcly3 wrote: post #85
Spivak wrote: post #83

Earlier quoted context omitted.

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

If you’re just committing for your own sake, that workflow sounds productive. I’ve been asked to review PRs with 20+ commits with a “wip” or “.” commit message with the argument: “it’ll be squash merged, so who cares!”. I’m sure that works well for the author, but it’s not great for the reviewer. Breaking change sets up into smaller logical chunks really helps with comprehension. I’m not generally a fan of people being cavalier with my time so they can save their own.

For my part, I find the “local history” feature of the JetBrains IDEs gives me automatic checkpoints I can roll back to without needing to involve git. On my Linux machines I layer in ZFS snapshots (Time Machine probably works just as well for Macs). This gives me the confidence to work throughout the day without needing to compulsively commit. These have the added advantage of tracking files I haven’t yet added to the git repo.

Re: Pre-commit hooks are broken

#87

Post by anttiharju »

I think the examples given in the post are just done poorly.

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.

Re: Pre-commit hooks are broken

#88

Post by sunshowers »

cluckindan wrote: post #41
geon wrote: post #32

Earlier quoted context omitted.

To get a commit history that makes sense. It’s not supposed to document in what order you did the work, but why and how a change was made. when I’m knee deep in some rewrite and realize I should have changed something else first, I can just go do that change, then come back and rebase. And in the feature branches/merge requests, I don’t merge, only rebase. Rebasing should be the default workflow. Merging adds so many…

That is just not true. Merging is so much less work and the branch history clearly indicates when merging has happened. With rebasing, there could be a million times the branch was rebased and you would have no idea when and where something got broken by hasty conflict resolution. When conflicts happen, rebasing is equivalent to merging, just at the commit level instead of at branch level, so in the worst case, devel…

Do you know what criss-cross merges are and why they're bad?

Re: Pre-commit hooks are broken

#89

Post by andrewaylett »

ltbarcly3 wrote: post #85
Spivak wrote: post #83

Earlier quoted context omitted.

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

I think you might like https://www.jj-vcs.dev/ — it snapshots before every operation, and can watch the filesystem to snapshot every change.

Re: Pre-commit hooks are broken

#90

Post by andrewaylett »

Spivak wrote: post #83
andrewaylett wrote: post #81

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…

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.

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.

Post reply on HN