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

141–150 of 178 posts

Re: Pre-commit hooks are broken

#141

Post by cluckindan »

sunshowers wrote: post #88
cluckindan wrote: post #41

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…

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

I’m sure you’re here to educate me, but this is not about criss-cross merges between two different work branches, this is about whether it’s better to rebase a work branch onto the main branch, or to pull the changes from the main branch to the work branch.

Re: Pre-commit hooks are broken

#142

Post by normie3000 »

thomashabets2 wrote: post #136
badgersnake wrote: post #123

Earlier quoted context omitted.

Then what’s the point? Just leave them off and run the tests when you want to run them.

Because 99% of my commits are not WIP commits. So I almost always want to run them. Hell, even most WIP commits will pass the tests (e.g. tests are not yet added for the new code), so I'd run them then too.

Some people write tests first.

Re: Pre-commit hooks are broken

#143

Post by jghn »

nirvdrum wrote: post #129
jghn wrote: post #119

Earlier quoted context omitted.

There are two halves here. Up until the PR is open, the author should feel free to have 20+ "wip" commits. Or in my case "checkpoint". However, it is also up to the author to curate their commits before pushing it and opening the PR. So when I open a Pr, I'll have a branch with a gajillion useless commits, and then curate them down to a logical set of commits with appropriate commit messages. Usually this is a single…

I clean up commits locally as well. But, I really only commit when I think I have something working and then collapse any lint or code formatting commits from there. Sometimes I need to check another branch and am too lazy to set up worktrees, so I may create a checkpoint commit and name it a way that reminds me to do a `git reset HEAD^` and resume working from there. But, if you're really worried about losing 15 min…

I don't commit as a backup. I commit for other reasons.

Most common is I'm switching branches. Example use case: I'm working locally, and a colleague has a PR open. I like to check out their branch when reviewing as then I can interact with their code in my IDE, try running it in ways they may not have thought of, etc.

Another common reason I switch branches is that sometimes I want to try my code on another machine. Maybe I'm changing laptops. Maybe I want to try the code on a different machine for some reason. Whatever. So I'll push a WIP branch with no intention of it passing any sort of CI/CD just so I can check it out on the other machine.

The throughline here is that these are moments where the current state of my branch is in no shape, way, or form intended as an actual valid state. It just whatever state my code happened to be in before I need to save it.

Re: Pre-commit hooks are broken

#144

Post by mycall »

normie3000 wrote: post #15
eru wrote: post #11

Earlier quoted context omitted.

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

You can an an enterprise environment when following SOPs are mandatory due to cybersecurity and infrastructure requirements.

Re: Pre-commit hooks are broken

#145

Post by pojzon »

aljgz wrote: post #139
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…

I'm the type of developer who always have a completely different way of working. I hate pre-commit hooks, and agree that pre-push + early step is CI is the right thing to do.

You dont have to install hooks. Its that simple.

Be prepared to have your PR blocked tho.

Re: Pre-commit hooks are broken

#146

Post by beezlewax »

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…

You can run git commit with a --no-verify flag to skip these hooks

Re: Pre-commit hooks are broken

#148

Post by sunshowers »

cluckindan wrote: post #141
sunshowers wrote: post #88

Earlier quoted context omitted.

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

I’m sure you’re here to educate me, but this is not about criss-cross merges between two different work branches, this is about whether it’s better to rebase a work branch onto the main branch, or to pull the changes from the main branch to the work branch.

I have an early draft of a blog post about them :) as a source control expert who built both these systems and tooling on top of them for many years, I think they're the biggest and most fundamental reason rebases/linear history are better than merges.

> whether it’s better to rebase a work branch onto the main branch, or to pull the changes from the main branch to the work branch.

The problem with this is that the latter has an infinitely higher chance of resulting in criss-cross merges than the former (which is 0).

Re: Pre-commit hooks are broken

#149

Post by souperdeluxe »

Without pre-commit you should squash on merge. That way main is atomic. But squashing loses granularity. A single merge commit might both refactor a function and implement a feature. You could split it into two pull requests, but maybe the feature depends on the refactor. Now your feature branch needs to cherry pick from the refactor branch. What if the refactor branch makes further changes prior to being merged? Now your feature branch has conflicts.

With pre-commit you can disable squashing on merge. You'll have a refactor commit and a feature commit. Changes to the refactor can be --fixup followed by rebase. I find this much easier than juggling two pull requests.

I accept that this is only viable with buy-in from everyone on the team, and I wouldn't advise it for teams over 10 people. But for small teams, I love pre-commit and trunk-based development.

Re: Pre-commit hooks are broken

#150

Post by patmorgan23 »

QuercusMax wrote: post #127
johnisgood wrote: post #125

Earlier quoted context omitted.

I know, I know, I was going to edit it to "Git{Hub,Lab}" in the beginning but oh well. In any case, my comment just reflects on the fact that you had a series of patches that you could not squash or rebase. It stuck. And the fact that I see many people use the abbreviation "PR" for something that is merely a patch or diff. For example you might send a diff to the tech@ mailing list, but you should not refer to it as…

Git{Hu,La}b

GitPub
Post reply on HN