Live data from Hacker News

Pre-commit hooks are broken

jyn.dev

41–50 of 178 posts

Re: Pre-commit hooks are broken

#41
post #32
post #22

why do people rebase so often? shouldn't it be excluded from the usual workflows as you are losing commit history as well?

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, developers are met with conflict after conflict, which ends up being a confusing mental burden on less experienced devs and certainly a ”trust the process” kind of workflow for experienced ones as well.

Re: Pre-commit hooks are broken

#42
post #37
post #30

Earlier quoted context omitted.

Isn't your tale more about squashing than rebasing?

Any subsequent commits and the branch are inherently rebased on the squashed commit. Rebasing is kind of a short hand for cherry-picking, fixing up, rewording, squashing, dropping, etc. because these things don't make sense in isolation.

I guess my point is that I disagree that rebasing should be shorthand for all these things that aren't rebasing.

Re: Pre-commit hooks are broken

#43
post #35
post #25

Earlier quoted context omitted.

so basically, not adhering to atomic commits. That's fine if it's a deliberate choice, but some people like me think commits should stand on their own. (i'm assuming your are not squashing when merging, else it's pretty much the same workflow)

Honestly, i find that a really weird view. I use (Local) commits for work in progress. I feel like insisting on atomic commits in your local checkout defeats the entire purpose of using a tool like git. What do you do when you are working on something and are forced to switch to working on something else in the middle of it?

I interpreted the parents post as: as long as my combination of commits results in something working before getting merged, it's fine.

Local wip commits didn't come to mind at all

Re: Pre-commit hooks are broken

#45
This article is very much "you're holding it wrong"

> They tell me I need to have "proper formatting" and "use consistent style". How rude.

> Maybe I can write a pre-commit hook that checks that for me?

git filter is made for that. It works. There are still caveats (it will format whole file so you might end up commiting changes that are formatting fixed of not your own code).

Pre-commit is not for formatting your code. It's for checking whether commit is correct. Checking whether content has ticket ID, or whether the files pass even basic syntax validation

> Only add checks that are fast and reliable. Checks that touch the network should never go in a hook. Checks that are slow and require an update-to-date build cache should never go in a hook. Checks that require credentials or a running local service should never go in a hook.

If you can do that, great! If you can't (say it's something like CI/CD repo with a bunch of different language involved and not every dev have setup for everything to be checked locally), having to override it to not run twice a year is still preferable over committing not working code. We run local checks for stuff that make sense (checking YAML correctness, or decoding encrypted YAMLs with user key so they also get checked), but the ones that don't go remote. It's faster. few ms RTT don't matter when you can leverage big server CPU to run the checks faster

Bonus points, it makes the pain point - interactive rebases - faster, because you can cache the output for a given file hash globally so existing commits during rebase take miliseconds to check at most

> Don't set the hook up automatically. Whatever tool you use that promises to make this reliable is wrong. There is not a way to do this reliably, and the number of times it's broken on me is more than I can count. Please just add docs for how to set it up manually, prominantly featured in your CONTRIBUTING docs. (You do have contributing docs, right?)

DO set it up automatically (or as much as possible. We have script that adds the hooks and sets the repo defaults we use). You don't want new developer to have to spend half a day setting up some git nonsense only to get it wrong. And once you change it, just rerun it

Pre-push might address some of the pain points but it doesn't address the biggest - it puts the developer in a "git hole" if they have something wrong in commit, because while pre-commit will just... cancel the commit till dev fixes it, with pre-push they now need to dig out knowledge on how to edit or undo existing commits

Re: Pre-commit hooks are broken

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

Re: Pre-commit hooks are broken

#47
post #22

why do people rebase so often? shouldn't it be excluded from the usual workflows as you are losing commit history as well?

If it is something like repo for configuration management I can understand that because its often a lot of very small changes and so every second commit would be a merge, and it's just easier to read that way.

... for code, honestly no idea

Re: Pre-commit hooks are broken

#48
post #12
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.

In our case same hook is re-ran on server side; the pre-commit hook is purely to increase velocity

... and cos most people using git will have to take a second if the hook returns to them "hey, your third commit is incorrect, you forgot ticket number"

Re: Pre-commit hooks are broken

#49
post #35
post #25

Earlier quoted context omitted.

so basically, not adhering to atomic commits. That's fine if it's a deliberate choice, but some people like me think commits should stand on their own. (i'm assuming your are not squashing when merging, else it's pretty much the same workflow)

Honestly, i find that a really weird view. I use (Local) commits for work in progress. I feel like insisting on atomic commits in your local checkout defeats the entire purpose of using a tool like git. What do you do when you are working on something and are forced to switch to working on something else in the middle of it?

> What do you do when you are working on something and are forced to switch to working on something else in the middle of it?

`git stash` is always an option :) but even if you want to commit it, you can always undo (or `--amend`) the commit when you get back to working. I personally am also a big fan of `git rebase -i` and all the things it allows me to fix up in the history before merging (rebasing) in to the main branch.

Re: Pre-commit hooks are broken

#50
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 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…

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.
Post reply on HN