Good test... but I don't think it properly portrays the way we (ok, me I
guess) really do development most of the time.
When we're creating a "feature branch"... that is typically some new
orthogonal feature that no one else is working on. I make all kinds of
commits... many of which just say "this compiles"... then, at the end I
come around and squash most of those into one or two commits that represent
individual chunks of work... but I don't usually care if they all "work" or
not. What matters is whether the new feature works... and I test that at
the end. As soon as something passes tests I put in a pull request for it.
Let's take a look at Ben's recent communicator deal. We were all working
on it... but for most of the time we were working on it you couldn't solve
any actual problem with that branch. Many of the commits in there are
broken. But it doesn't matter. What matters is that at the end of the day
the new feature works.
Ben, could have been merging from master all along... but it wouldn't have
mattered (because the code didn't really work anyway). Instead, at the end
he rebased it all and just stacked it on the end of master... tested to
make sure that it worked and then pushed.
Essentially... here is my point: If you do a local commit of something new
and you test it and it works... push it out! Don't sit around on tons of
patches that are tested and working while stuff changes out from underneath
you. Put in a pull request (or just push it!) and then keep working. When
it gets merged you can rebase your branch on that and keep working.
This is really not any different from someone sitting around with tons of
changes in their local SVN copy. They've been working on it for weeks and
testing along the way and everything is looking good... but they just keep
developing and sitting on it. Everything is fine until someone comes along
and changes "trunk" out from underneath them and now suddenly all of their
stuff is broken. If they would have been committing to trunk all along
then that wouldn't have happened.
Whether you are doing merging or rebasing the advice is still the same:
Don't sit on patches and let master change out from under you!
Roy: The other good test to do is one that involves a collision. In the
merge case you will get weird bits in that merge commit that come from the
thing it collided with (even if it was a few patches ago). In the rebase
case you get to modify the one patch in your sequence that collides and
each patch is then whole and complete.
Derek
On Mon, Apr 8, 2013 at 2:40 PM, Roy Stogner <roystgnr@...:
>
> If anybody is curious about what this looks like in practice, there's
> an example now at https://github.com/roystgnr/**testgit<https://github.com/roystgnr/testgit>
>
> The Cliff's Notes version: we end up with breakage either way, but
> more breakage with rebase.
>
> With merges:
> 1: Start with merge_master
> 2: Branch off merge_branch
> 3: Make changes to merge_master, making sure everything works before
> each commit.
> 4: Make subtly incompatible changes to merge_branch, making sure
> everything works before each commit.
> 5: "git pull" from master to branch. No conflicts are created, but
> the result doesn't pass tests. A merge commit was created in the
> process of the pull, though, so we've got a commit in the history that
> doesn't work.
> 6: Fix the incompatibilities and commit the fix, push things, blah
> blah; we can still be left with one broken commit per merge in the
> project history.
>
> With rebasing:
> 1-4: as before, but with "rebase_master" and "rebase_branch"
> 5: "git pull --rebase" from master to branch. No conflicts or merge
> commits are created. The uncommitted result doesn't pass tests...
>
> but more importantly, every previous branch commit no longer passes
> the tests that it previously passed! Try "git checkout 2b718" or "git
> checkout 77e5c", run make, and watch the tested-fine-until-the-rebase
> commits all fail.
> 6: as before, but now instead of "one broken commit per merge" we've
> got "N broken commits per rebase".
>
> Ideally I'd like "test everything before committing" to be a
> sufficient strategy for achieving zero broken commits, but I don't
> know how to git there from here.
> ---
> Roy
>
>
>
> On Mon, 4 Feb 2013, Derek Gaston wrote:
>
> On Mon, Feb 4, 2013 at 10:15 PM, Derek Gaston <friedmud@...> wrote:
>>
>> And the alternative rebased log looks like "A, then D, then D+(B-A),
>>
>> then D+(C-A)", right? And that's nice because it's more like what
>>
>> we'd get from passing patches around, but in fact that intermediate
>>
>> "D+(B-A)" state is one that never really existed and was never
>> actually tested.
>>
>>
>> A little more of a direct reply (just to be even more clear). What the
>> rebased log looks like
>> (in your scenario): A, D, B, C (with the way you named them).
>>
>> Just before the rebase you had this scenario:
>>
>> upstream/master: A, D
>> roys_feature: A, B, C
>>
>> When you do a rebase (git pull --rebase upstream master) you end up with:
>>
>> upstream/master: A, D
>> roys_feature: A, D, B1, C1
>>
>> ie you took your _new_ patches and "stacked" them on top of what was
>> currently sitting in
>> master. If there were any collisions with D you fixed them up during the
>> rebase... so that's
>> why B and C (possibly) changed to B1 and C1. They might be slightly
>> different from the B and C
>> you started with. This is normal and familiar to us though. The same
>> thing happens when you do
>> "svn up" and fix collisions. Your local work has been modified somewhat
>> because of what someone
>> did in trunk.
>>
>> Now you quickly rerun your testing for B1 and C1 (just like with SVN) and
>> verify that everything
>> is still good and then you push:
>> git push upstream HEAD:master
>>
>> Meaning:
>>
>> upstream/master: A, D, B1, C1
>> roys_feature: A, D, B1, C1
>>
>> No merge commits get created. All collisions are dealt with _per patch_
>> which means that there
>> are no extraneous bits of commits mashed together in merge commits...
>> each patch is whole and
>> complete and applies cleanly.
>>
>> This means that if I pull and start working with master I get:
>>
>> dereks_feature: A, D, B1, C1
>>
>> And if I want to undo C1 for some reason I can simply do:
>>
>> git reset HEAD~1 --hard
>>
>> To get:
>>
>> dereks_feature: A, D, B1
>>
>> I don't have to worry about what happens when I undo a merge commit (does
>> the code even compile
>> in that case? Are things just somewhat broken?). I can move _linearly_
>> back through the
>> history in master with impunity. No messy merge commits to step around
>> or try to understand.
>>
>> Derek
>>
>>
|