Comment by lmm | Hacker News Reader

Comment by lmm | Hacker News Reader

Parent Full thread

> git-bisect works best when every commit works, contains a single idea, and stacks in a linear history. That works best in a publicly visible branch, and is why it is helpful to squash an entire pull-request into a single, atomic commit — one which clearly defines the change from before- to after-this-feature.

Disagree; git-bisect works best when every commit is small and most commits work (in particular, as long as any broken commit is likely to have a neighbour that works - isolated bad commits aren't a problem (that's what skip is for, and it's easy enough to include that in your script - you do automate your bisects, right?), long chains of bad commits are). Squashing means your bisect will land on a squashed commit, when it's only really done half the job. (In particular, the very worst case, where every single one of your intermediate commits was broken, is the same as the case you get when you squash)

> When it’s ready for the attention of your peers then you absolutely ought to dress it up as smartly as possible. It’s at that point that you write a cover letter for your change: what was the situation before, why that was bad, what this patch does instead, and how you proved in practice that it made things better (tests!)

And that's what the PR description is for! You don't have to destroy all your history to make one.

Thanks for responding. Everything you say I agree with. I think our differences lie in the scope of how much of my private activity do I want to share in public.

You’re right that GitHub, GitLab et al let you use their tooling to write the final commit message (for the merge commit or squash commit). My preference has always been to do that in git itself.

In both cases you end up with a single atomic commit that represents the approved change and its description. For me, the commit is created the moment a review is requested, instead of from the moment it is approved and landed. One reason this is particularly useful is that you can now treat the commit as if it had already landed on the main branch. (It is easier to share, cherry-pick, rebase, etc. — easier than doing so with a branch of many commits, in my experience.)

Prospective changes do not change type (from branch to squashed commit or merge commit) either, when they are approved, which simplifies these workflows.

You can also use just plain git and make sure the merge-commit has a useful message, while leaving the work-branch unsquashed and available to explore and bisect when necessary. The main branch looks as neat as when using squashes, and using something like git log --merges --first-parent all the small commits on the work-branches are hidden anyway. It looks just like when using atomic commits, but the extra details are still there when someone needs them.

I've followed your same approach but switched to having just "most of the commits work" after I found out about git bisect --skip.

> One reason this is particularly useful is that you can now treat the commit as if it had already landed on the main branch. (It is easier to share, cherry-pick, rebase, etc. — easier than doing so with a branch of many commits, in my experience.)

> Prospective changes do not change type (from branch to squashed commit or merge commit) either, when they are approved, which simplifies these workflows.

You can do that even earlier if you simply never squash or otherwise edit history, which is my approach - any pushed feature branch is public as far as I'm concerned, and my colleagues are encouraged to pull them if they're e.g. working on the same area at the same time. It comes at the cost of having to actually revert when you need to undo a pushed commit (and, since cherry-pick is not an option, if you're making e.g. a standalone fix and want your colleagues to be able to pull it in unrelated branches, you have to think ahead a little and make it on a new branch based from master rather than sticking it in the middle of your incomplete feature branch), but it's very much worth it IME.

Can't git-bisect simply ignore commits with a commit message smaller than N characters?

That might sound facile, but it's actually a great idea. Being able to ignore commits based on regexes would be even more powerful.

Not sure it really has huge benefits, but I guess something like this should work:

``` #!/bin/sh N=20 msg_len=$(git log -1 --pretty=%B | wc -c) if [ "$msg_len" -lt "$N" ]; then exit 125 fi # Here you would run your actual test and report 0 (good), 1 (bad) as needed exit 0 ```

Yes - or, being more principled, you can tell it to only use mainline commits (first parent) if landing on a whole PR is what you want.

Reply on news.ycombinator.com