Thanks a ton! That was very educational.
On Fri, Sep 25, 2015 at 2:22 AM, Chris Hillery <[email protected]> wrote: > So, I've done some more experimentation with Gerrit's various merge > strategies. > > First a reminder that a Gerrit review is always of a single commit, not a > branch. This massively limits our ability to share branches before we even > consider the Gerrit merge strategy, because it forces us to squash changes > down to single commits. If we truly want to "solve" that problem, we'll > need a much more extensive change to working method than just the Gerrit > merge strategy. Also, any method that solved that problem and still had > meaningful code reviews would involve reviewers needing to individually > review every commit from the developer's branch. In all honesty, it might > be easier to use a completely different review tool than Gerrit at that > point. > > Anyway, at the risk of drowing this conversation completely... there are > five merge strategies available in Gerrit; here are what I see as their > pros, cons, and gotchas. > > 1. *Cherry-pick*. This is what we use today: the commit under review is > cherry-picked onto master, resulting in a new commit. > > Pros: > - Introduces the "Reviewed-on" header in the commit on master, which > allows linking back to the review page. > - Ensure linear master commit history, since there are no merge commits. > Cons: > - Cherry-picks are always new commits (although it seems that rebase and > merge can handle that OK - the real damage has already been done by the > squashing before it even gets to Gerrit) > > 2. *Always merge*. This means that the commit under review will be put into > the repository, and then a merge commit will be created (by Gerrit itself) > with the current master tip as one parent and the new commit as another. > Basically the same as using "git merge --no-ff" locally. > > Pros: > - Original commit maintains SHA, etc. > - Original commit does not necessarily have to be rebased to master tip > (although if it isn't there can be merge conflicts) > Cons: > - Master history can be a bit messy (although "git log --first-parent" > works reasonably well) > - The commit message Gerrit creates is always: ' *Merge "first line of > commit message"* ' which means the master commit history loses a ton of > information. > > 3. *Fast-forward only*. This means that the commit under review must have > the current master tip as its parent. When the commit is submitted, Gerrit > will simply fast-forward the master branch to the new commit. Basically the > same as using "git merge --ff-only" locally. > > Pros: > - Original commit maintains SHA, etc. > - Master log history is kept very clear. > - No possibility of merge conflicts at Gerrit time. > Cons: > - Since you must rebase to master before submitting, you potentially lose > the ability to share your working branch with others more often than is > strictly necessary. > - Also since you must rebase to master before submitting, you will > potentially have more submissions that need to be rebased and reproposed > than is strictly necessary. > > 4. *Merge if necessary*. This is basically the same as the default > behaviour of "git merge": fast-forward if possible (ie, the commit under > review's parent is the current master tip), otherwise create a merge > commit. > > Pros: > - Good compromise between "keeping local branch commits shareable" and > "don't force unnecessary rebases" - possibly the least developer-intrusive > option. > Cons: > - Master history is inconsistent, with a mix of "real" commits and merge > commits. > - Merge commits still have the extremely abbreviated log message, which > is IMHO even worse here since some commits will have full messages and some > won't. > > 5. *Rebase if necessary*. This will fast-forward master if the commit under > review has the current master tip as its parent. If not, Gerrit itself will > attempt to rebase the commit onto master, and then fast-forward it onto > master. This may result in conflicts at Submit time. > > Pros: > - Less developer interaction required than "fast-forward only". > - Master log history is kept very clear. > Cons: > - Commits on master will sometimes be new commits, basically cherry-picks > (without the advantages of cherry-pick) > > > Anyway, I'm not sure any of this information is useful. There isn't a merge > strategy which is clearly better or worse than any other, and the problem > of wanting more flexibility in development branches is only tangentially > related to that anyway. But, here's the information in case anyone wants to > think about it any more. > > I'm going to do a little bit more exploration with "git gerrit" to see > whether I could change anything there that would make a difference. > > Ceej > aka Chris Hillery > > On Thu, Sep 24, 2015 at 5:52 PM, Chris Hillery <[email protected]> > wrote: > > > On Thu, Sep 24, 2015 at 11:05 AM, Jianfeng Jia <[email protected]> > > wrote: > > > >> If we can somehow enforce the rebase before merge, then I think it won’t > >> have the merge “tree” problem. > >> > > > > Actually the merge-tree problem was introduced in a situation where a > part > > of a branch was cherry-picked to master. It would also happen if part of > a > > branch were cherry-picked over to another branch (like you wanted to do > > with Taewoo's branch) and then both were eventually merged to master. In > a > > normal scenario this wouldn't happen, with or without rebasing the > feature > > branch before merge. > > > > As for whether "rebase before merge" should be enforced, I'm in favor of > > it personally. I'm even in favor of doing local interactive rebases with > > selective squashing and so forth to "clean up" local branch history > before > > merging - although as always, this is destructively rewriting history and > > therefore should not be done if the branch was shared with anyone else. > > > > Ceej > > aka Chris Hillery > > >
