Can gerrit handle reviewing and merging entire topic branches with multiple commits

3,661 views
Skip to first unread message

dbailey

unread,
Sep 28, 2011, 12:54:54 PM9/28/11
to Repo and Gerrit Discussion
Is it possible to submit a series of commits and have gerrit treat
them as a branch where the tip is merged into the target branch once
all commits are approved?

I can fudge this behaviour in a manner:

Set project settings to "Always Merge"

Given
HEAD (master)
\
A --> B --> C (feature/A)

Doing 'git push gerrit refs/for/master/feature/A' from the feature/A
branch and then reviewing them in the order of C, B and finally A will
result in gerrit creating a merge commit D as in the following

HEAD -------------> D (master)
\ /
A --> B --> C

With a commit message along the lines of:
Merge changes change_id C,change_id B,change_id A into master

* changes:
ChangeC commit subject
ChangeB commit subject
ChangeA commit subject

This however only works provided I review the commits in the order C,
B then A. If I review A, B and then C, they each get merged as
separate commits.


I can also manage this if I change the settings to fast-forward only
and do the merge commit myself before submitting. Again if I review in
the order of D->A, everything works as expected, non of the commits
appear until all are reviewed and submitted.

If I review in the order A->D, things work fine provided the reviews
are all completed straight away before anything else is reviewed.
Otherwise it will be applied in between some of the commits and break
the latest ones. I actually don't like this method at all, to easy for
things to go wrong.


The standard approach appears to be squash all the commits and submit
as a single change to be reviewed. Which for small bugs and minor
changes is just fine. However we've found for new features that being
able to see the series of commits is extremely useful (devs are
encouraged to squash changes into logical sets).


So, I'm wondering if gerrit can be configured in such a way as to
enforce the behaviour of the first example, where it will wait until
all 3 commits are reviewed and then merge the tip to the target
branch, irrespective of the order of the reviews? Basically review
entire branches and merge the tip once fully reviewed.

If not, if anyone has any suggestions as to an alternative way of
using gerrit that provides a similar behaviour.

Are there any issues logged that request this behaviour? I had a look
and didn't see any?

Magnus Bäck

unread,
Sep 29, 2011, 2:31:57 AM9/29/11
to Repo and Gerrit Discussion
On Wednesday, September 28, 2011 at 18:54 CEST,
dbailey <dba...@hp.com> wrote:

> Is it possible to submit a series of commits and have gerrit treat
> them as a branch where the tip is merged into the target branch once
> all commits are approved?
>
> I can fudge this behaviour in a manner:
>
> Set project settings to "Always Merge"
>
> Given
> HEAD (master)
> \
> A --> B --> C (feature/A)
>
> Doing 'git push gerrit refs/for/master/feature/A' from the feature/A
> branch and then reviewing them in the order of C, B and finally A will
> result in gerrit creating a merge commit D as in the following
>
> HEAD -------------> D (master)
> \ /
> A --> B --> C

In Git, HEAD is the currently checked out ref (or commit, in the
detached head case). Using that term like this doesn't really make
sense.

> With a commit message along the lines of:
> Merge changes change_id C,change_id B,change_id A into master
>
> * changes:
> ChangeC commit subject
> ChangeB commit subject
> ChangeA commit subject
>
> This however only works provided I review the commits in the order C,
> B then A. If I review A, B and then C, they each get merged as
> separate commits.

You can *review* them in any order you like, but you need to *submit*
them in topological order (C, B, A).

Apart from aesthetics, why is it a problem if the your branch will look
like this?


... ---- D --> E --> F (master)
\ / / /


A --> B --> C

If commits A and B can't function without C (i.e. they must be merged
atomically), should they really be separate commits?

Or are you after the ability to choose only once the feature is done
whether to merge it to the destination branch? That's our main reason
for using feature branches similar to what you've described -- don't
pollute the branch until we know the feature is done and meets the
quality goals. We solve this with manual merge commits.

[...]

> The standard approach appears to be squash all the commits and
> submit as a single change to be reviewed. Which for small bugs
> and minor changes is just fine. However we've found for new
> features that being able to see the series of commits is extremely
> useful (devs are encouraged to squash changes into logical sets).

I don't know from where you get the idea that squashing multiple
logical changes into single commits is in any way regarding as a
standard approach.

> So, I'm wondering if gerrit can be configured in such a way as to
> enforce the behaviour of the first example, where it will wait until
> all 3 commits are reviewed and then merge the tip to the target
> branch, irrespective of the order of the reviews? Basically review
> entire branches and merge the tip once fully reviewed.

Some people have been working on a topic review feature that might help
you. It was discussed in a recent thread. There's also ongoing work to
make atomic sets of commits across different gits, but I don't think
that'll help you either.

> If not, if anyone has any suggestions as to an alternative way of
> using gerrit that provides a similar behaviour.
>
> Are there any issues logged that request this behaviour? I had a look
> and didn't see any?

I don't think anything like this is planned. What would be the user
interface for a feature like this? How would you instruct Gerrit to
merge from the tip of a topic? What if a topic has multiple entry
points? I don't think this would be a technicaly difficult thing to
implement, but the user interface and semantics is non-obvious to me.

--
Magnus Bäck Opinions are my own and do not necessarily
SW Configuration Manager represent the ones of my employer, etc.
Sony Ericsson

Bailey, Darragh

unread,
Sep 29, 2011, 11:44:33 AM9/29/11
to repo-d...@googlegroups.com

On 29/09/11 07:31, Magnus Bäck wrote:
> You can *review* them in any order you like, but you need to *submit*
> them in topological order (C, B, A).
True, so is there anyway to enforce a particular order for submission?

> Apart from aesthetics, why is it a problem if the your branch will look
> like this?
>
>
> ... ---- D --> E --> F (master)
> \ / / /
> A --> B --> C
>
> If commits A and B can't function without C (i.e. they must be merged
> atomically), should they really be separate commits?
>
> Or are you after the ability to choose only once the feature is done
> whether to merge it to the destination branch? That's our main reason
> for using feature branches similar to what you've described -- don't
> pollute the branch until we know the feature is done and meets the
> quality goals. We solve this with manual merge commits.
>
> [...]

This and making it easy to see all commits that are requires to
implement a feature should it be necessary to cherry-pick to a another
branch. Also didn't want someone to be half way through and submit the
initial commits for merging and then realise that there were problems
with subsequent commits and end up with a half of a feature going in.

>> The standard approach appears to be squash all the commits and
>> submit as a single change to be reviewed. Which for small bugs
>> and minor changes is just fine. However we've found for new
>> features that being able to see the series of commits is extremely
>> useful (devs are encouraged to squash changes into logical sets).
> I don't know from where you get the idea that squashing multiple
> logical changes into single commits is in any way regarding as a
> standard approach.

Sorry, would have been better said as the "approach I've come across so
far is to squash all the commits". I haven't seen any setups where
people have been using gerrit with topic branches and managed it in
other ways.


>> So, I'm wondering if gerrit can be configured in such a way as to
>> enforce the behaviour of the first example, where it will wait until
>> all 3 commits are reviewed and then merge the tip to the target
>> branch, irrespective of the order of the reviews? Basically review
>> entire branches and merge the tip once fully reviewed.
> Some people have been working on a topic review feature that might help
> you. It was discussed in a recent thread. There's also ongoing work to
> make atomic sets of commits across different gits, but I don't think
> that'll help you either.

I think I finally found the discussion thread
http://groups.google.com/group/repo-discuss/browse_thread/thread/e6846b6bfb4b28c0

Had to go through a few pages to find it, should really have switched to
the new groups view before now, much easier to find it.


>> If not, if anyone has any suggestions as to an alternative way of
>> using gerrit that provides a similar behaviour.
>>
>> Are there any issues logged that request this behaviour? I had a look
>> and didn't see any?
> I don't think anything like this is planned. What would be the user
> interface for a feature like this? How would you instruct Gerrit to
> merge from the tip of a topic? What if a topic has multiple entry
> points? I don't think this would be a technicaly difficult thing to
> implement, but the user interface and semantics is non-obvious to me.

I'm not sure either exactly how it should look, but it looks like
someone has made a stab at implementing what I'm interested in:
https://github.com/petefoth/gerrit-topic-reviews

Also found the issue for me to track
http://code.google.com/p/gerrit/issues/detail?id=51

--
Regards,
Darragh Bailey

Torne Wuff

unread,
Sep 29, 2011, 1:01:14 PM9/29/11
to Bailey, Darragh, repo-d...@googlegroups.com
On 29 September 2011 16:44, Bailey, Darragh <dba...@hp.com> wrote:
> On 29/09/11 07:31, Magnus Bäck wrote:
>> You can *review* them in any order you like, but you need to *submit*
>> them in topological order (C, B, A).
> True, so is there anyway to enforce a particular order for submission?

It is enforced.. if you submit a change, it also submits all the
changes it depends on (unless they are not reviewed, in which case it
will not submit any of them). You only need to hit submit on C, in
this case. (unless you use the cherry-pick submit policy, which
negates all this logic and lets you submit changes out of order as
long as they are cherrypickable).

--
Torne Wuff
to...@wolfpuppy.org.uk

Reply all
Reply to author
Forward
0 new messages