Live data from Hacker News

Pre-commit hooks are broken

jyn.dev

61–70 of 178 posts

Re: Pre-commit hooks are broken

#61
post #30
post #26

Earlier quoted context omitted.

I write really poopy commit messages. Think "WIP" type nonsense. I branch off of the trunk, even my branch name is poopy like feature/{first initial} {last initial} DONOTMERGE {yyyy-MM-dd-hh-mm-ss} Yes, the branch name literally says do not merge. I commit anything and everything. Build fails? I still commit. If there is a stopping point and I feel like I might want to come back to this point, I commit. I am violentl…

Isn't your tale more about squashing than rebasing?

[deleted]

Re: Pre-commit hooks are broken

#62
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?

The sum of the re-written changes still amount to the same after a rebase. When would you need access to the pre-rebase history, and to what end?

Well, sometimes you do if you made a mistake, but that's already handled by the reflog.

Re: Pre-commit hooks are broken

#63
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?

really; keep reading about all the problems ppl have “every time I rebase” and I wonder what tomfoolery they’re really up to

Unlike some other common operations that can be easily cargo-culted, rebasing is somewhat hard to do correctly when you don't understand git, so people who don't understand git get antagonistic towards it.

Re: Pre-commit hooks are broken

#64

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

> they now need to dig out knowledge on how to edit or undo existing commits

This knowledge is a crucial part of effective use of git every day, so if some junior dev has to learn it quick it's doing them a favor.

Re: Pre-commit hooks are broken

#65
post #53
post #43

Earlier quoted context omitted.

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

Well we are in a discussion about pre-commit hooks. Pre-commit hooks run on local wip commits.

Well, unless you inhibit them with `-n`. Which I would for WIP commits.

Re: Pre-commit hooks are broken

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

The master branch never gets merged, so it is linear. Finding a bug is very simple with bisect. All commits are atomic, so the failing commit clearly shows the bug.

If you want to keep track of what commits belongs to a certain pr, you can still have an empty merge commit at the end of the rebase. Gitlab will add that for you automatically.

The ”hasty conflict resolution ” makes a broken merge waaaay harder to fix than a broken rebase.

And rebasing makes you take care of each conflict one commit at a time, which makes it order by magnitudes easier to get them right, compared to trying to resolve them all in a single merge commit.

Re: Pre-commit hooks are broken

#67
post #44

The pre-commit framework [1] abstracts all these issues away and offers a bunch of other advantages as well. [1]: https://pre-commit.com/

the pre-commit framework does not abstract away “hooks shouldn’t be run during a rebase”, nor “hooks should be fast and reliable”, nor “hooks should never change the index”.

Not sure how you got to that conclusion, as the pre-commit framework does indeed abstract them away. Maybe you're confusing it with something else?

> hooks shouldn’t be run during a rebase

The pre-commit framework doesn't run hooks during a rebase.

> hooks should be fast and reliable

The pre-commit framework does its best to make hooks faster (by running them in parallel if possible) and more reliable (by allowing the hook author to define an independent environment the hook runs in), however it's of course still important that the hooks themselves are properly implemented. Ultimately that's something the hook author has to solve, not the framework which runs them.

> hooks should never change the index

As I read it the author says hooks shouldn't change the working tree, but the index insteead and that's what the pre-commit framework does if hooks modify files.

Personally I prefer configuring hooks so they just print a diff of what they would've changed and abort the commit, instead of letting them modify files during a commit.

Re: Pre-commit hooks are broken

#68
post #66

Earlier quoted context omitted.

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…

The master branch never gets merged, so it is linear. Finding a bug is very simple with bisect. All commits are atomic, so the failing commit clearly shows the bug. If you want to keep track of what commits belongs to a certain pr, you can still have an empty merge commit at the end of the rebase. Gitlab will add that for you automatically. The ”hasty conflict resolution ” makes a broken merge waaaay harder to fix th…

Linear history is nice, but it is lacking the conflict resolutions. They are never committed, and neither are the ”fix rebase” instances.

Having a ”fix broken merge” commit makes it explicit that there was an issue that was fixed.

Rebase sometimes seems like an attempt at saving face.

Re: Pre-commit hooks are broken

#69
post #67

Earlier quoted context omitted.

the pre-commit framework does not abstract away “hooks shouldn’t be run during a rebase”, nor “hooks should be fast and reliable”, nor “hooks should never change the index”.

Not sure how you got to that conclusion, as the pre-commit framework does indeed abstract them away. Maybe you're confusing it with something else? > hooks shouldn’t be run during a rebase The pre-commit framework doesn't run hooks during a rebase. > hooks should be fast and reliable The pre-commit framework does its best to make hooks faster (by running them in parallel if possible) and more reliable (by allowing th…

> Ultimately that's something the hook author has to solve, not the framework which runs them.

correct. i'm saying that hook authors almost never do this right, and i'd rather they didn't even try and moved their checks to a pre-push hook instead.

Re: Pre-commit hooks are broken

#70
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?

I don't want to see any irrelevant history several years later, so I enforce linear history on the main branch in all projects that I work on. So far, nobody complained, and I've never seen a legitimate reason to deviate from this principle if you follow a trunk based release model.
Post reply on HN