Open Stack

Jeremy Stanley <fungi at yuggoth.org> writes:

> On 2013-04-23 16:56:29 -0700 (-0700), Spencer Judge wrote:
> [...]
>> Is there a way I can push just the merge, with all the changes,
>> as one review (on master)? Am I just missing something? Or is
>> this a workflow that isn't really supported by the tool?
>
> Apologies for the late reply, but we just realized we hadn't been
> keeping up with the moderator queue for the ML. No good. :/
>
> It seems (having managed Gerrit for OpenStack and interacted with
> some other projects' Gerrit servers as well) that it's uncommon for
> the "Push Merge Commit" permission to be allowed into
> refs/for/refs/* by default, so the client assumes a rebase-oriented
> workflow instead. Also, I'm not entirely certain how Gerrit would
> treat a merge commit with a Change-Id: header (if you have any
> public examples of this in action I'm curious to see what it looks
> like and how it treats the other parent commits).
>
> I suspect, though I certainly could be mistaken, that even if
> git-review grew this feature it still wouldn't match the sort of
> workflow Gerrit expects anyway.

Yes, the basic issue is that Gerrit is designed so that one 
commit == one change in Gerrit.  And in order for a change/commit to 
be merged into a branch in Gerrit, its parent commits/changes must be
merged as well.  This holds true for merge commits as well, actually, so
you _can_ use merge commits in gerrit, but only after the whole branch
individually has been merged.

For instance, we do use this technique, rarely, in OpenStack for feature
branches.  If a project has branch "feature/foo" and branch "master",
and people are using Gerrit to merge individual changes/commits into
both of them, then you can simply create a merge commit and propose that
to Gerrit, and it can be reviewed in the normal way.  Here's an example:

  https://review.openstack.org/#/c/29260/

Because both branches already have their individual changes/commits
merged in Gerrit, just that merge commit needs to be reviewed.  Note
that it is empty, because there are no actual changes, it's just a merge
commit (nothing more).

However, yes, the idea of 'create a local branch of several commits and
push the whole branch to be reviewed as one change' does not work with
Gerrit.  The closest analog we think is the rebase workflow -- where you
create a branch locally, possibly with several commits, and then when
you are ready for it to be reviewed, squash the branch into one commit
and submit that for review.  You can still keep the local branch for
your own history, but you're asking reviewers to review one logical
change.  Of course, if your change has several logical steps, then go
ahead and push all the commits to Gerrit -- that's fine.

The reason merge commits are usually not allowed in Gerrit is because
it's very easy to accidentally create local merge commits when pulling
from other branches and then push hundreds of commits to Gerrit.  This
happened a few times in our early use of Gerrit and led to our current
thinking of how to develop locally and push changes.

You may find the following documentation useful in understanding how we
came to these conclusions:

  https://wiki.openstack.org/wiki/GerritJenkinsGithub
  https://wiki.openstack.org/wiki/GerritWorkflow

>> If there's anything I can do to contribute, or even add this
>> functionality myself, just point me at it and I'll see what I can
>> do.
>
> We're always thrilled to have help improving our software! Find us
> in #openstack-infra on the Freenode IRC network if you want to chat,
> or follow up to this mailing list (we're going to try to do a better
> job of keeping up with moderation requests, but it's low-volume so
> if you subscribe you don't have to worry about moderation anyway).

Thanks,

Jim