Hacker News Reader

Comment by Mithaldu | Hacker News Reader

It's absolutely resolved that way.

Rebase should be the weapon of choice in all cases, except those where it would be prohibitively expensive.

People are too enamored with the SVN ways though, or have even made up rationalizations about fictional concepts such as "historical purity". So you have a ton of people merging instead, producing code repositories that look worse than the worst perl code i've touched.

View on HN

> People have even made up rationalizations about fictional concepts such as "historical purity".

The idea behind not rewriting history is not to gain some kind of academic pleasure from purity. It's to make sure that the commits being committed are actually the same ones that you actually wrote and (hopefully) found to be working, so that when a bug makes you refer to the commit later, you (a) don't start scratching your head as to why it didn't do what you thought you saw it do yesterday, and (b) can actually refer to it to figure out what the correct code should've looked like.

If you don't see what I mean, here's an example. Say you have

  foo():
      foo1()
      foo2()
      foo4()
      foo5()

and you turned it into this:

  foo():
      foo1()
      foo2()
      foo4()
      foo5()
      foo6()

Now now if someone has in the meantime done this:

  foo():
      foo1()
      foo2()
      if (!foo3()): return 0
      foo4()
      foo5()

then after a rebase you will end up with

  foo():
      foo1()
      foo2()
      if (!foo3()): return 0
      foo4()
      foo5()
      foo6()

which is not code that you ever had, and which is likely to fail because your assumption that foo6() will always be hit will no longer be valid.

With a rebase, not only will this be confusing when you look back and make you blame the wrong person or lead you in the wrong direction, but it will also remove the option of rolling back to the latest of either of the two branches that you combined. You'll only be able to revert to the "known-good" branch that was rebased onto, not the one that was rebased.

Oh, and did I mention you very likely just lost the working (pre-rebase) version of your code entirely? Have fun digging it out of your reflog if you notice it quickly enough, and good luck finding a copy if you do so too late. You'll have probably deleted the branch by now, and chances are nobody else will have a copy lying around for no reason either. After some period of time you'll have no idea what the working code looked like, because the only version you have is the one that already incorporated the branch you rebased onto.

Life sucks sometimes but it doesn't matter whether you merge or rebase in the scenario you describe, you need to test the combined version of the code. You can't just assume it is still working and whoever comes second needs to shoulder that responsibility.

> Life sucks sometimes but it doesn't matter whether you merge or rebase in the scenario you describe, you need to test the combined version of the code. You can't just assume it is still working and whoever comes second needs to shoulder that responsibility.

Sure you do. But you're missing the fact that testing is in no way even remotely guaranteed to catch every error. If it did then projects that have tests would never have bugs, and that doesn't seem to even approximate the world we live in. (Why did I need to point this out though? Is this not obvious?)

Sorry for being a bit vague but I didn't mean to imply there were tests. I meant test in the sense of "do whatever you did to check correctness" in the first place. You change the code, you should do that process again or at least some approximation of it. Automated tests make it pretty easy, manual testing makes it doable, stepping through with a debugger or dumping printing statements makes it painful and error prone.

Many codebases don't have tests at all, most have terrible tests, and under no circumstances do I think they magically prevent all bugs. But I do think obnoxious coders who assume they can just merge and move on without checking the correctness have a special place in hell.

> I meant test in the sense of "do whatever you did to check correctness" in the first place.

Sure. I already responded to this:

>> But you're missing the fact that testing is in no way even remotely guaranteed to catch every error.

Obviously you have to test the combination but the merging allows an easier rollback.

Easier? How so? If a rebase of a branch isn't successful you just reset back to the commit you rebased onto, and continue work on your old pre-rebase branch.

But if the idea is that I want `foo6()` etc to always be executed, wouldn't I just handle the rebase from master by resolving the conflict as follows, instead?

  foo():
      foo1()
      foo2()
      foo4()
      foo5()
      foo6()
      if (!foo3()): return 0

> wouldn't I just handle the rebase from master by resolving the conflict

(a) No, because there is no conflict. It gets merged "cleanly" without you realizing it. (Let me emphasize this: merge is NOT guaranteed give a correct result if it happens cleanly. This is just one example.)

(b) No, obviously in general doing foo3() at the end will not give the same result as doing it in the middle.

It would be nice if git kept both versions of the history around. One with work in progress commits, corrections from review, and merges. The other a clean rebased history for usual history browsing.

You can do something like that easily by keeping multiple branches around. I do that sometimes when I'm doing some major rebase/cleanup of commits spanning more than a couple of days. Just so afterwards I can compare what I end up with with what I had originally.

Your example is in fact a good example why you should always rebase before merge. Only then would the reviewer have a chance to see the (potential) problem before it is merged.

I'm not sure I understand why rebasing would confuse blame here. Have you tried it?

If you rebase your "foo6" change on top of the "!foo3" change, the state of your branch will be exactly the state of your code base going forward. And that is the useful history to keep, not the order in which the actual keys were pressed.

Conversely, people get too enamored of rebasing as a way to solve the problem of merging in lots of junky commits.

There are lots of ways to rewrite history and clean it up. Rebasing is just one of them, and it's one that can force you to spend more time resolving conflicts than you otherwise might have to (since you have to replay all of your commits and resolve any conflicts caused by each one individually).

There's no reason why rebasing is a better way to clean up your history than squash merging (or even -- carefully -- using git reset).

I already said that it can be too expensive, but thanks for playing. <3

Also, a squash merge is not a merge. It's just another form of rebase.

And if you mention reset as an alternative to cherry pick and squash you should also mention --soft.

My point was that many git users don't understand git all that well, and I think that encouraging everyone to rebase is a bad idea.

If you're going to promote a particular workflow as the the one that "should be the weapon of choice," I think the workflow of merge-master-into-feature-branch, squash-merge-feature-branch-into-master is a more user friendly alternative, since then the cases where rebasing is expensive never even come up.

And yes, you're right that I was remiss not to mention the --soft part. The git reset workflow is one that I would definitely not encourage people to do if they don't understand git well.

> Rebase should be the weapon of choice [....] People are too enamored with the SVN ways though

I'd expect SVN users to be quite comfortable with rebasing. `git pull --rebase` is effectively the same thing as `svn update`. You just couldn't commit unfinished work when using SVN, so the changes were loose in the working tree, rather than stored in local commits.

Reply on news.ycombinator.com