News for hackers. Stuff that matters.
Re:Show HN: Hiding code from Git diff by stinos on Wednesday December 13, 2017 @05:26PM (#15918283)
Isn't this sort of solved by rebasing first instead of just merging? (well, and some rules in place which tells people 'we only accept properly rebased branches for review). Then the conflicts are resolved in the branch and there's a clear view of what actually changed in the diff, before anything is effectively merged into the main branch. Or maybe I'm missing something here.
Re:Show HN: Hiding code from Git diff by Mithaldu on Wednesday December 13, 2017 @06:00PM (#15918516)
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.

Re:Show HN: Hiding code from Git diff by mehrdadn on Wednesday December 13, 2017 @08:03PM (#15919159)
> 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.

Re:Show HN: Hiding code from Git diff by dummyfunnytoo on Wednesday December 13, 2017 @10:36PM (#15919756)
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.
Re:Show HN: Hiding code from Git diff by mehrdadn on Wednesday December 13, 2017 @10:59PM (#15919837)
> 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?)

Re:Show HN: Hiding code from Git diff by dummyfunnytoo on Thursday December 14, 2017 @04:10AM (#15920912)
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.

Re:Show HN: Hiding code from Git diff by mehrdadn on Thursday December 14, 2017 @01:29PM (#15925043)
> 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.

Re:Show HN: Hiding code from Git diff by fahrradflucht on Thursday December 14, 2017 @12:50AM (#15920224)
Obviously you have to test the combination but the merging allows an easier rollback.
Re:Show HN: Hiding code from Git diff by Mithaldu on Thursday December 14, 2017 @05:36AM (#15921259)
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.
Re:Show HN: Hiding code from Git diff by xorcist on Thursday December 14, 2017 @07:18AM (#15921693)
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.

Re:Show HN: Hiding code from Git diff by grkvlt on Thursday December 14, 2017 @12:09AM (#15920086)
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
Re:Show HN: Hiding code from Git diff by mehrdadn on Thursday December 14, 2017 @01:46AM (#15920398)
> 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.

Re:Show HN: Hiding code from Git diff by orblivion on Thursday December 14, 2017 @01:24AM (#15920336)
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.
Re:Show HN: Hiding code from Git diff by stinos on Thursday December 14, 2017 @04:11AM (#15920917)
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.
Re:Show HN: Hiding code from Git diff by lemoncucumber on Wednesday December 13, 2017 @06:54PM (#15918810)
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).

Re:Show HN: Hiding code from Git diff by Mithaldu on Wednesday December 13, 2017 @07:07PM (#15918888)
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.

Re:Show HN: Hiding code from Git diff by lemoncucumber on Wednesday December 13, 2017 @07:44PM (#15919058)
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.

Re:Show HN: Hiding code from Git diff by slavik81 on Thursday December 14, 2017 @02:02AM (#15920451)
> 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.

Re:Show HN: Hiding code from Git diff by neilparikh on Wednesday December 13, 2017 @09:16PM (#15919469)
I don't think you need to use rebase solve this issue at all. All you need to do is to make sure is that every commit on master, is on your feature branch, which you can still do with merging. Something like:

    git checkout my-branch
    git merge master
Whether you use rebase or merge is a completely orthogonal issue.
Re:Show HN: Hiding code from Git diff by orblivion on Thursday December 14, 2017 @01:20AM (#15920319)
Maybe I'm misunderstanding here. Who is going to run `git merge master`? If it's not you, the maintainer, then you need to trust that the submitter didn't pull any funny business with that command. With rebasing, you can just confirm that there are no merge commits.