Re: [RFC/PATCH] log: add log.firstparent option

From: Jeff King <peff@peff.net>
Date: Jul 24, 2015, 07:21 UTC
Message-ID: <20150724072109.GB2111@peff.net>
In-Reply-To: <xmqqpp3io5km.fsf@gitster.dls.corp.google.com>

On Thu, Jul 23, 2015 at 03:46:33PM -0700, Junio C Hamano wrote:

Show 16 quoted lines

> While I can see the reasoning behind the above [*1*], I am not sure
> if the output with "--first-parent" would always be a good match for
> the "simple" version.  Wouldn't the people who keep these messy
> histories also those who merge project trunk into a random topic and
> push the result back as the updated project trunk?  Admittedly, that
> merely is saying that "--first-parent" is not a solution to such a
> project, and does not say much about the usefulness of the new
> configuration, so I'd queue it as-is.
> 
> [Footnote]
> 
> *1* I do not necessarily agree, though.  The history being messy is
>     a sign that "the actual thought process of the developer" was
>     not clearly expressed in the work, and it is not likely that you
>     have more insight by looking at it---you will see more mess, for
>     certain, though.

I don't think people who keep "messy" commit histories are necessarily prone to pushing to the trunk. And ditto for the footnote, I do not think "messy" is necessarily a bad thing. It is really about whether you choose to rebase out follow-on fixups (e.g., those that come from review), or whether you choose to leave them in.

And there is a very good reason to leave them in: two people collaborating on a topic have a much easier time if the other person is not constantly rebasing.

So a GitHub-oriented workflow can be something like (and this is what we use to develop GitHub internally):

  1. I do the usual process of creating a branch, writing some commits,
     pushing them up, and opening a pull request. This is equivalent to
     git.git, except the final step is "send to the list". I may use
     "rebase -i" during this process to make a nice sequence of commits.

  2. During review, I may get comments. In a git.git workflow, I am
     expected to re-roll the topic myself to address the comments. If I
     am lucky, the reviewer expressed their comments as a diff, and I
     can squash them in as appropriate. But in the GitHub workflow, the
     reviewer may simply check out the branch themselves locally, commit
     the fixes, and push them up. They become part of the PR.

  3. I may go back and forth with the reviewer several times, and there
     may be intermingled commits from both of us. At the end, one of us
     decides to merge the PR, and GitHub's merge button creates a non-ff
     commit on master, merging in the topic.

So we're "messy" in the sense that we chose to record the back-and-forth during the development of the topic. But at no point was anybody in danger of back-merging and putting the result directly onto master. Even if they did back-merge from master onto the topic[1], the non-ff merge to master means that any first-parent traversal starting from master will never stray onto a PR branch at all.

Of course not everybody does this. But I think it's a perfectly sane workflow. The PR merges become the most interesting unit of change to review or find in the history, with the individual commits closer to "what really happened".

-Peff

[1] We do actually back-merge master to topics all the time, because we
    have to make sure that we whatever we test and deploy contains all
    of what is already in master, even if the topic was originally
    forked from an older point. IOW, the QA process for merging is
    basically:

      - merge back from master so we know the tree at the tip of our
	topic is what _would_ be on master if we merged

      - test that tip (with automated tests, temporarily deploying to
	the live site, etc)

      - it's OK to merge to master if it has not moved; the merge would
        be a fast-forward. Instead, we create a merge commit, but the
	tree content will be the same (so we know that we are merging
	and deploying what has just passed QA).

    Incidentally, doing "git log --first-parent" while on the topic
    branch from this workflow does the reasonable thing. Because the
    merges on the topic branch are all backwards merges from master,
    following the first parent keeps you on the topic branch (rather
    than showing you all the upstream commits you just pulled in).

    Where I'd be most worried about a default first-parent hurting you
    is when you "git log $some_random_commit", at which point you may or
    may not want first-parent behavior. I suppose we could have a config
    option that kicks in only when using the default "HEAD". That seems
    more likely to do what you want all the time, but it also sounds
    rather confusingly magical.