Hacker News

Yeah, bisect is really handy because often a bug will have been introduced as a side-effect of a change made to support something else, and if you don't know what new usage was introduced you're relatively likely to break that in the course of fixing the bug.

You can avoid it via the "just look at every usage of this function and hold the entire codebase in your head" method, of course, but looking at the commit seems a bit simpler.

> With git bisect, all of sudden a normal user was able to bisect the kernel by himself and point to the concrete commit (and dev) that broke things.

Huh. Thanks for pointing that out. I definitely would never have thought about the use case of "Only the end user has specific hardware which can pinpoint the bug."

Yes to both of these. In a healthcare setting, some bugs leave data that needs to be reviewed and/or corrected after it is identified and fixed.

And also a fair number of bugs filed can be traced back to someone asking for it to work that way.

See also Two Devs Breaking Each Other’s Features.

I got to hear about a particularly irate customer during a formative time of my life and decided that understanding why weird bugs got in the code was necessary to prevent regressions that harm customer trust in the company. We took too long to fix a bug and we reintroduced it within a month. Because the fix broke another feature and someone tried to put it back

https://www.reddit.com/r/talesfromtechsupport/s/K2xme9A0MQ

I once bisected to find a bug in a 6 month old commit. An off-by-one error in some array processing. I fixed the bug there to confirm. But on main, the relevant code didn’t even exist any more. It had been completely refactored away.

I ended up rebasing the entire 6 months worth of commits onto the bugfix, propagating the fix throughout the refactoring.

Then a diff against main showed 3 lines changed in seemingly unrelated parts of the code, together triggering the bug. I would never have found them without bisect and rebase.

Every system is imperfectly understood.

I don’t use it for myself often, but I use it fairly often when someone has to escalate a problem to me. And how you work when the shit hits the fan says a lot about you overall, IMO.

You need to specify diff format, so that Git can correctly identify and parse function body.

*.py diff=python

The overflow bug wasn't fixed until the 21st century; the comment you remember reading dates from before it was discovered.

To be fair, in most computing environments, either indices don't overflow (Smalltalk, most Lisps) or arrays can never be big enough for the addition of two valid array indices to overflow, unless they are arrays of characters, which it would be sort of stupid to binary search. It only became a significant problem with LP64 and 64-bit Java.

Agreed.

> Using `mid = (low + high) / 2` has an overflow bug if you're not using infinite precision, but there are several other potential problems even in the simplest algorithm.

Well, if you are doing binary search on eg items you actually hold in memory (or even disk) storage somewhere, like items in a sorted array (or git commits), then these days with 64 bit integers the overflow isn't a problem: there's just not enough storage to get anywhere close to overflow territory.

A back of the envelope calculation estimates that we as humanity have produced enough memory and disk storage in total that we'd need around 75 bits to address each byte independently. But for a single calculation on a single computer 63 bits are probably enough for the foreseeable future. (I didn't go all the way to 64 bits, because you need a bit of headroom, so you don't run into the overflow issues.)

Yes, it's a simple enough algorithm to be a good basic exercise---most people come up with binary search on their own spontaneously when looking a word up in dictionary.

Property based testing is really useful for finding corner cases in your binary search. See eg https://fsharpforfunandprofit.com/series/property-based-test... for one introduction.

Crafting a PR as an easily-consumed, logical sequence of commits is particularly useful in open source.

1. It makes review much easier, which is both important because core maintainer effort is the most precious resource in open source, and because it increases the likelihood that your PR will be accepted.

2. It makes it easier for people to use the history for analysis, which is especially important when you may not be able to speak directly to the original author.

These reasons also apply in commercial environments of course, but to a lesser extent.

For me, organizing my PRs this way is second nature and only nominal effort, because I'm extremely comfortable with Git, including the following idiom which serves as a more powerful form of `git commit --amend`:

    git add -p
    git commit --fixup COMMIT_ID
    git stash
    git rebase -i --autosquash COMMIT_ID~

> For that matter, why merge? Rebase it on top.

Yes, that works for me although it might not work for people who aren't going to the same lengths to craft a logical history. I have no interest in preserving my original WIP commits — my goal is to create something that is easy to review.

BUT... the PR should ultimately be merged with a merge commit. Then when you have a bug you can run `git bisect` on merges only, which is good enough.

> Can someone explain why anyone would want non-squashed PRs? > > For the 5% of engineers that diligently split each PR into nice semantic changes, I suppose that's nice. But the vast majority of engineers don't do this.

I think cause and effect are the other way around here. You write and keep work-in-progress commits without caring about changes because the history will be discarded and the team will only look at pull requests as a single unit, and write tidy distinct commits because the history will be kept and individual commits will be reviewed.

I've done both, and getting everyone to do commits properly is much nicer, though GitHub and similar tools don't really support or encourage it. If you work with repository history a lot (for example, you have important repositories that aren't frequently committed to, or maintain many different versions of the project) it's invaluable. Most projects don't really care about the history—only the latest changes—and work with pull-requests, which is why they tend to use the squashed pull request approach.

> For the 5% of engineers that diligently split each PR into nice semantic changes, I suppose that's nice. But the vast majority of engineers don't do this.

Here's a simple reason: at my company, if you don't do this, you get fired.

This is basic engineering hygiene.

> If you truly care about making more semantic history, split the work into multiple PRs.

This exactly - if your commit history for a PR is interesting enough to split apart, then the original PR was too large and should have been split up to begin with.

This is also a team culture thing - people won't make "clean" commits into a PR if they know people aren't going to be bisecting into them and trying to build. OTOH, having people spend time prepping good commits is potentially time wasted if nobody ever looks at the PR commit history aside from the PR reviewers.

> For that matter, why merge? Rebase it on top. It's so much cleaner. It's atomic and hermetic.

With an explicit merge, you keep two histories, yet mostly care about the "main" one. With rebase, you're effectively forgetting there ever was a separate history, and chose to rewrite the history when "effectively merging" (rebasing).

There's value in both, mostly seems to come down to human preference. As long as the people that will be working with it agrees, I personally don't care either way which one, granted it's consistently applied.

> Each PR being squashed to 1 commit is nice and easy to reason about. If you truly care about making more semantic history, split the work into multiple PRs.

I don't argue with your point (even if I am obsessive about commits separation), but one needs to keep in mind that the reverse also applies, that is, on other end of the spectrum, there are devs who create kitchen-sink PRs which include, for example, refactorings, which make squashed PRs harder to reasons about.

> Can someone explain why anyone would want non-squashed PRs?

So you can differentiate the plumbing from the porcelain.

If all the helpers, function expansions, typo corrections, and general renamings are isolated, what remains is the pure additional functional changes on its own. It makes reviewing changes much easier.

Making human-readable commit history is not that hard with a little practice. It's one of the big benefits of tools like magit or jj. My team started doing it a few weeks ago, and it's made reviewing PRs substantially easier.

If you ever worked with stacked PRs, and the top one gets squashed and merged it often becomes a nightmare to rebase the rest of the PRs to bring them up to date.

git merge --no-ff

git log --first-parent

git bisect --first-parent

The above gives you clean PR history in the main branch while retaining detailed work history in (merged) feature branches.

I really don't understand why would I squash having git merge --no-ff at my disposal...

I would definitely neither accept a pull request where the individual commits were testing and iteration, nor a pull request with hundreds of lines of changes are in a single commit. (New code, maybe.) It's not about ceremony; it's about knowing what changed and why.

Hoo boy is it fun to figure out where things went wrong when the real commit history was thrown away to make it look prettier. Especially a mistake from a merge conflict.

I'd take fully squashed PRs over endless "fix thing" and "updated wip"... but if you work in a way that leaves a couple meaningful commits, that's even better. Sometimes I end up in this state naturally by having a feature branch, which I work on in sub branches, each being squashed into a single final commit. Or when the bulk of the logic is on one commit, but then a test case or two are added later, or a configuration needs changing.

I like merge commits because they preserve the process of the review.

I wrote a text about that:

https://lucasoshiro.github.io/posts-en/2024-04-08-please_don...

I have always been very careful with git histories and often rewrite/squash them before final review/merge. Often my rewritten histories have nothing to do with the original history and commits are logically/intuitively separated and individually testable.

That said, very few people seem to be like me. Most people have no concept of what a clear commit history is. I think it's kind of similar to how most people are terrible written communicators. Few people have any clue how to express themselves clearly. The easiest way to deal with people like this is to just have them squash their PRs. This way you can at least enforce some sanity at review and then the final commit should enforce some standards.

I agree on rebasing instead of straight merging, but even that's too complicated for most people.

You can just inspect merge commits, you can also just bisect over merge commits.

Splitting work into multiple PRs is unnecessary ritual.

I have never reasoned about git history or paid attention to most commit messages or found any of it useful compared to the contents of the change.

When I used git bisect with success it was on unknown projects. Known projects are easy to debug.

Because github doesn't support stacked diffs, basically.

T_T

I manage a team of developers, and I don't think any of us squash commits, and I really don't care. It's been working fine for 8 years at this job.

We keep our git use extremely simple, we don't spend much time even thinking about git. The most we do with git is commit, push, and merge (and stash is useful too). Never need to rebase or get any deeper into git. Doing anything complicated with git is wasting development time. Squashing commits isn't useful to us at all. We have too much forward velocity to worry that some commit isn't squashed. If a bug does come up, we move forward and fix it, the git history doesn't really figure into our process much, if at all.

There is no free lunch, the same people that can't be bothered to make atomic semantic commits are the same people that will ruin your bisect with a commit that doesn't build or has some other unrelated run failure. People that don't care can't be fixed by tools.

The advice around PRs rings hollow, after all they were invented by the very people that don't care - which is why they show all changes by default and hide the commits away, commit messages buried after 5 clicks. And because this profession is now filled with people that don't care, add the whole JIRA ticket and fix version rigmarole on top - all kinds of things that show up in some PMs report but not in my console fixing an issue that requires history.

“tues lunch wip”

I can think of only a couple of cases over 20+ years where I had to bisect the commit history to find a bug. By far the normal case is that I can isolate it to a function or a query or a class pretty quickly. But most of my experience is with projects where I know the code quite well.

Important addendum to 4 that can throw someone their first time - Put the new test in a new file and don't commit it to the repo yet. You don't want it to disappear or conflict with old versions of the test file when bisect checks old commits.

Worse: PVCS!

Or visual source safe

and sccs

Including "code is delivered in a way that involves merges"