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

171–178 of 178 posts

Re: Pre-commit hooks are broken

#171

Post by WorldMaker »

sunshowers wrote: post #170
WorldMaker wrote: post #167

Earlier quoted context omitted.

rerere is used automatically by git to cache certain merge conflict fixes encountered during a rebase so that you don't have to reapply them more than once rebasing the same branch later. In general, when it works, which is most of the time, it's part of what keeps rebases feeling easy and lightweight despite capturing in the final commit output sometimes a fraction of the data of a real merge commit. The rerere cach…

You have to turn rerere on, though, right? I've never done that. I've also never worked with long-running branches — tend to strongly prefer integrating into main and using feature flags if necessary. Jujutsu doesn't have anything like rerere as far as I know.

Hmm, yeah looks like it is default off. Probably some git flow automation tool or other sort of bad corporate/consultant disseminated default config at a past job left the impression that it was default on. It's the solution to a lot of papercuts working with long-running branches as well as the source of new problems as stated above; problems that are visible with merge commits but hidden in rebases.

Re: Pre-commit hooks are broken

#172

Post by mcv »

QuercusMax wrote: post #168
mcv wrote: post #155

Earlier quoted context omitted.

Never had conflicts on a merge? We've got a lot of people on the same codebase. Merge conflicts are a fact of life. And they wouldn't be a problem without the stupid commit hook. It's the commit hook that makes them a problem.

If you have conflicts then you can fix them and run your linter or formatter. If you have a no conflict merge it doesn't matter.

Thanks, but that's not the issue here.

Re: Pre-commit hooks are broken

#173

Post by baobun »

mcv wrote: post #116
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…

There's a weird thing happening on my current project. Sometimes I merge main into my branch and it fails. What fails is the pre-commit hook on the merge commit. Changes in main fail the linting checks in the pre-commit hook. But they still ended up in main, somehow. So the checks on the PR are apparently not as strict as the checks on the pre-commit hook. As a result, many developers have gotten used to committing w…

For the sake of argument, let's say you have a check that caps the number of lines per file and that both you and main added lines in the same file. It's not too weird if that check fails only after merge, right?

One benign example of something that can break after merge even if each branch is individually passing pre-merge. In less benign cases it will your branch merged to main and actual bugs in the code.

One reason to not allow "unclean merges" and enforced incoming branches to be rebased up-to-date to be mergable to the main branch.

You probably want to run the checks on each commit to main in CI and not rely on them being consistently run by contributors.

You do you but I find rebasing my branch on main instead of merging makes me scratch mybhead way less.

Re: Pre-commit hooks are broken

#174

Post by normie3000 »

thomashabets2 wrote: post #152
normie3000 wrote: post #142

Earlier quoted context omitted.

Some people write tests first.

And commit in such that the final timeline has broken tests for half of commits? Sounds like an awful way to live your life.

No, we're not talking about the final timeline. That is finalised when (or if) code is merged to the mainline. We're talking about what happens when the command "git commit" is executed.

Re: Pre-commit hooks are broken

#175

Post by normie3000 »

seba_dos1 wrote: post #60
normie3000 wrote: post #16

Earlier quoted context omitted.

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

Remember that such key will be copied into the repository on `git add` already and will stay there until garbage collected.

Thanks, good to keep in mind. Although presumably that means it's in my local repository. And it would have already been in my local filesystem anyway.

Re: Pre-commit hooks are broken

#176

Post by thomashabets2 »

normie3000 wrote: post #174
thomashabets2 wrote: post #152

Earlier quoted context omitted.

And commit in such that the final timeline has broken tests for half of commits? Sounds like an awful way to live your life.

No, we're not talking about the final timeline. That is finalised when (or if) code is merged to the mainline. We're talking about what happens when the command "git commit" is executed.

Ok, if you're talking about just WIP commits that will be squashed and will never be part of mainline, then shrug.

For me that's a tiny proportion of commits. I'd rather avoid taking a whole finished feature branch and then spend more time cleaning up a sloppy commit history.

Sure, sometimes it's correct to squash, but for nontrivial changes I go with https://github.com/google/eng-practices/blob/master/review/d...

Re: Pre-commit hooks are broken

#177

Post by nirvdrum »

ltbarcly3 wrote: post #156
nirvdrum wrote: post #110

Earlier quoted context omitted.

> But this would require hand curation? No development proceeds that way, or if it does then I would question whether the person is spending 80% of their day curating PRs unnecessarily. It's not really hand curation if you're deliberate about it from the get-go. It's certainly not eating up 80% of anyone's time. Structuring code and writing useful commits a skill to develop, just like writing meaningful tests. As a f…

Aha, I see the issue here. What you seem to organize into cute little self contained 'commit's I would put on individual 'branches'. It is too hard for you to get someone to look at a PR, so you are packing multiple 'related' but not interdependent changes into one PR as individual commits so you can minimize the number of times you have to get someone to hit "approve", which is the limiting resource. In your situati…

I'd encourage you to read the HN guidelines, linked in the site footer:

https://news.ycombinator.com/newsguidelines.html

Repeatedly, you've been dismissive and insulting. It's not conducive to productive conversation. Your characterization of what I do or how I work is wrong. You latched on to some small part you thought would let you "win" and ran with it. If you actually care, I do a lot of open source work so you can find exactly how I work. Naturally, you can't see what I do in private, but I assure you it's not significantly different.

I aim to ship reasonably complete functionality. The "V" in "MVP" means it needs to be viable, not just minimal. Shipping some part that doesn't work standalone isn't useful to anyone. Yes, the PR is smaller, but now the context for that work is split over multiple PRs, which may not be reviewed by the same people. No one really has the full picture beyond me, which I guess is a good way to get my PRs rapidly approved, but a terrible way to get feedback on the overall design.

I don't work with you so I don't particularly care how you work. Again, I was offering up other solutions than running "git commit" every 15 minutes. If you want to manually simulate filesystem snapshots, that's your prerogative. But, you're incorrect that any model other than the one you employ is niche an not how software is written. Elsewhere you dismissed the examples of large, open source projects as being unique. But, you'll find substantially smaller ones also employ a model closer to what I've described.

Re: Pre-commit hooks are broken

#178

Post by ltbarcly3 »

nirvdrum wrote: post #177
ltbarcly3 wrote: post #156

Earlier quoted context omitted.

Aha, I see the issue here. What you seem to organize into cute little self contained 'commit's I would put on individual 'branches'. It is too hard for you to get someone to look at a PR, so you are packing multiple 'related' but not interdependent changes into one PR as individual commits so you can minimize the number of times you have to get someone to hit "approve", which is the limiting resource. In your situati…

I'd encourage you to read the HN guidelines, linked in the site footer: https://news.ycombinator.com/newsguidelines.html Repeatedly, you've been dismissive and insulting. It's not conducive to productive conversation. Your characterization of what I do or how I work is wrong. You latched on to some small part you thought would let you "win" and ran with it. If you actually care, I do a lot of open source work so you…

You literally said to me "Structuring code and writing useful commits a skill to develop, just like writing meaningful tests." as a response to my reasonable comments.

Do you not see how ironic it is for you to then cry that I'm hurting your feelings by being dismissive and insulting?

Post reply on HN