asterixdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Hillery <chill...@hillery.land>
Subject Re: Merging vs. squashing
Date Fri, 25 Sep 2015 09:22:15 GMT
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 <chillery@hillery.land>
wrote:

> On Thu, Sep 24, 2015 at 11:05 AM, Jianfeng Jia <jianfeng.jia@gmail.com>
> 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
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message