asterixdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ted Dunning <ted.dunn...@gmail.com>
Subject Re: Merging vs. squashing
Date Fri, 25 Sep 2015 15:02:10 GMT
Thanks a ton!

That was very educational.



On Fri, Sep 25, 2015 at 2:22 AM, Chris Hillery <chillery@hillery.land>
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 <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