Mailing-List: contact dev-help@asterixdb.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@asterixdb.incubator.apache.org
MIME-Version: 1.0
In-Reply-To: 
 <CAHWyyRxv+EpzvybwFQLJR0wd5i=UuVc=HTqLd365UEpXj4LHCQ@mail.gmail.com>
References: 
 <CAHWyyRxv+EpzvybwFQLJR0wd5i=UuVc=HTqLd365UEpXj4LHCQ@mail.gmail.com>
Date: Fri, 18 Sep 2015 08:47:43 -0700
Message-ID: 
 <CANQf6Dq_Xm2TEjfpDFxTr1psctJ3zWC6fygZRJRBizNZBjWqMQ@mail.gmail.com>
Subject: Re: Merging vs. squashing
From: Chen Li <chenli@gmail.com>
To: dev@asterixdb.incubator.apache.org
Content-Type: text/plain; charset=UTF-8

I assume the conclusion is: we keep our current git practice.  Right?

Based on this practice, is there any easy way for people like Jianfeng
to make their merge into their branch simpler?  I think Young-Seok is
doing experiencing something similar to merge the master into his
one-year-behind geo branch.

Chen

On Fri, Sep 18, 2015 at 1:04 AM, Chris "Ceej" Hillery
<ceej@couchbase.com> wrote:
> I'm pulling this conversation into a separate thread from the "Commit
> message proposal".
>
> There are certainly long-running and contentious arguments about whether
> commits to master should be merges or squashes/cherry-picks. That ranks
> right up there with vi vs. emacs. Before going too far into that, however,
> you should be aware of some additional quirks/limitations that Gerrit
> brings into the story.
>
> First, we have Gerrit configured to always cherry-pick changes when
> submitting them. We do this for a couple of reasons, but the main one is
> that Gerrit gives you a little additional value in this case: It introduces
> Reviewed-on: and Reviewed-by: headers into the commit, which provide you
> with a link back to the Gerrit review. For instance,
> https://github.com/apache/incubator-asterixdb-hyracks/commit/d885779b13c4fd9bd551d306a460df2894fb18cb
> - see that there is a hyperlink there back to Gerrit. This is not possible
> if you use any merge strategy other than cherry-pick, because only
> cherry-picking creates a *new* commit object that Gerrit can decorate.
>
> Moving on - In Gerrit, it is not possible to review a "branch merge", at
> least not in the way you would hope. A review in Gerrit is always of a
> *single commit*. If you make a number of commits on your local working
> branch and then push that branch to Gerrit's refs/for/master, it will
> create one review for each commit. This is occasionally desirable for
> larger changes that can reasonably be broken up, but as a default working
> method it is usually more trouble than it's worth - especially because
> Gerrit won't prevent you from Submitting a subset of the changes or merging
> them out of order. This is the main reason that I implemented git-gerrit to
> do a squash before uploading a change to Gerrit.
>
> What if you make changes on a branch, then merge them to your local master
> and then propose just the merge commit to Gerrit? That's just one commit
> (the merge commit) and therefore one review, right? Well, no - that would
> only work if all of your branch commits were already known to Gerrit, say
> because they were submitted (one at a time) to a non-master branch.
> Otherwise, you'll get N+1 separate reviews on Gerrit, and the merge commit
> review won't even make sense anymore.
>
> It *is* possible to do this "right" with Gerrit if you want to get merge
> commits onto the master branch. You need to create a secondary branch,
> "unstable" or "testing" or similar, and propose your individual changes
> there. We could even create multiple of those branches for individual
> feature work if we wanted. Then once that branch is in a state that you
> want to merge to master, you can propose a single merge commit change to
> Gerrit. The biggest downside to this is that when reviewing a merge commit,
> Gerrit does NOT show you any information about the changes being introduced
> into the target branch. It only shows you any diffs which were introduced
> during conflict resolution of the merge. Also, merge commits cannot be
> cherry-picked, so you lose the Reviewed-on: header for the merge itself.
> These aren't necessarily fatal limitations - we have a couple teams in
> Couchbase using exactly this methodology with success.
>
> Anyway. The upshot is, handling git history is a mess, and Gerrit makes it
> messier. The "always squash branches, always cherry-pick changes" approach
> that we currently use for AsterixDB seems to me to be the least available
> evil, and it at least results in a clean if abbreviated commit history on
> master. It definitely has its downsides as well, though - Jianfeng's
> experience is one such problem, and it doesn't have a clean solution.
>
> Ceej
> aka Chris Hillery