How to work collaboratively with Gerrit?
Joel
I was wondering how people work collaboratively with Gerrit?
Just say I have 2 or more people working on a feature and it's not
ready for a code review. But the 2 developers need to share code with
each other before changes have been merged into master (via a code
review in Gerrit).
I was thinking you could allow them to push up to a special dev branch
inside gerrit, so they can push their commits into gerrit without the
need for a code review, so basically a regular remote git repository.
Which I suppose could be done with "Push Branch" "refs/heads/dev-*" or
something to that effect?
Alternately we could setup a remote repository somewhere so that team
can use to share, but then we'd have to setup accounts etc, where if
it's in gerrit all the ssh authentication is nicely managed.
Or I guess the members of the team can pull from each others
repositories, if they can figure out how to get ssh working properly.
But that makes me nervous because if they work internally for 2 or 3
weeks without a code review, and then something happens to their
computers all the work is lost.
And also using that approach a few days before a release, all the work
for the last month will arrive as 20,000 lines in one commit, which
would take ages to review, and give hardly any time for changes.
On the other side of the coin if commits are pushed up all the time
with all the commits being dependent on each other, it's really quite
hard to go and edit a previous commit I've found. And you generally
have to approve the changeset and then get them to fix their changes
in another changeset. Or you abandon all the changesets, get them to
squash all the commits together and approve that one.
Android is a huge project so how do people work there? Are all the
android committers git ninjas and have no problem sharing code with
each other before a code review?
Any ideas welcome.
Cheers,
-Joel
Rob Heittman
Shawn Pearce
> I also have found the "DO NOT MERGE" subject hint really valuable for using
> Gerrit to stow code in progress or just share a patchset with another
> developer, without too much fear of landing the thing by mistake.
Yea, this is a good strategy. Android itself uses "DO NOT MERGE" to
actually mean something else, these show up in the history as
submitted changes but are messages to Google's merge bot to *not*
carry the change from a branch like donut into master automatically.
Developers tag their commits with "DO NOT MERGE" when they know there
is a nasty merge conflict that needs to be dealt with by hand.
Personally I use "EDIT" or "PARK" as the first word of a commit
message to mean "this is not done and still has more to do". Often
I'll then leave notes to myself in the body of the commit message...
but not always. It might just be the single line "EDIT query stuff"
or whatever.
Shawn Pearce
> I was wondering how people work collaboratively with Gerrit?
Like with Git itself we have multiple ways available, your team needs
to identify what will work for them.
> Just say I have 2 or more people working on a feature and it's not
> ready for a code review. But the 2 developers need to share code with
> each other before changes have been merged into master (via a code
> review in Gerrit).
>
> I was thinking you could allow them to push up to a special dev branch
> inside gerrit, so they can push their commits into gerrit without the
> need for a code review, so basically a regular remote git repository.
>
> Which I suppose could be done with "Push Branch" "refs/heads/dev-*" or
> something to that effect?
Yes, this is one reason why we support the Push Branch feature. It
lets you use Gerrit as a normal Git server, so the SSH configuration
can still be leveraged for other repository management even if the
code review functionality isn't needed.
Actually the pattern "dev-*" won't be honored as a wildcard. In 2.1.3
and earlier this has to be "refs/heads/dev/*" and thus your branch
name must be "dev/work" or "dev/add-that-feature". If you use the
bleeding edge master (or 2.1.4 when its released) you can use a
regular expression match "^refs/heads/dev-.*".
Within Google (and at least one other company using Gerrit) we have
recently setup a bleeding edge master version of Gerrit Code Review
and are using the personal sandbox branch concept:
Push Branch +3 refs/sandbox/${username}/*
Code Review +2 refs/sandbox/${username}/*
Submit +1 refs/sandbox/${username]/*
This gives each developer their own area to work in. But its
primarily meant for backing up your laptop or your workstation
overnight, or to help you move a patch between the two before its
ready to get sent to a reviewer. We aren't really using it as an area
for peer collaboration.
Personally, I prefer creating a branch, doing the code review on the
branch as you develop the code, and then merging the branch into the
mainline when its ready. If two developers are collaborating on a
change together they should be able to peer-review each other's change
as they are being written, and then submit into that branch. When it
comes time to merge the branch into the trunk, its already all been
reviewed, so all that is left is to review the merge.
> On the other side of the coin if commits are pushed up all the time
> with all the commits being dependent on each other, it's really quite
> hard to go and edit a previous commit I've found.
git rebase -i. Its not that hard these days. Its a bit annoying.
And sometimes it just isn't worth it.
For example in JGit recently we have a series of over 100 commits
being developed, with a lot of code refactoring going on. If an issue
was identified during review in say the 5th commit in the series,
fixing it just isn't worth it because the entire section of code might
be gone by the 30th commit. But we're mostly talking about debatable
style issues here ("this method should be purple not blue") and not
logic correctness ("this should be <= not <").
> And you generally
> have to approve the changeset and then get them to fix their changes
> in another changeset. Or you abandon all the changesets, get them to
> squash all the commits together and approve that one.
I try to avoid doing this. I force them to learn how to use git
rebase -i to go back and fix their change.
> Android is a huge project so how do people work there? Are all the
> android committers git ninjas and have no problem sharing code with
> each other before a code review?
They aren't git ninjas... or well, they weren't when they started.
But they did wind up spending some time to learn the tools. But the
reality is, code reviews happen in near real time. Developers get a
code review and then immediately submit the change to the branch. If
the change(s) have to stay out of the branch for a period of time, a
temporary branch is created for them to develop, review and submit on,
and then that branch is merged into the master branch when the work is
complete. Those who have really learned how to use Git will juggle
stacks of commits 2-5 deep, but if you get too far ahead of your
reviewers there is no hope of you getting your code submitted.
Joel
> git rebase -i. Its not that hard these days. Its a bit annoying.
> And sometimes it just isn't worth it.
>
> For example in JGit recently we have a series of over 100 commits
> being developed, with a lot of code refactoring going on. If an issue
> was identified during review in say the 5th commit in the series,
> fixing it just isn't worth it because the entire section of code might
> be gone by the 30th commit. But we're mostly talking about debatable
> style issues here ("this method should be purple not blue") and not
> logic correctness ("this should be <= not <").
>
> > And you generally
> > have to approve the changeset and then get them to fix their changes
> > in another changeset. Or you abandon all the changesets, get them to
> > squash all the commits together and approve that one.
>
> I try to avoid doing this. I force them to learn how to use git
> rebase -i to go back and fix their change.
commit, you would get the author to rebase all the 100 commits? And
then you'd abandon the 100 previous changesets and just approve the
new mega commit?
Obviously abandoning 100 changesets would be annoying, so I guess
you'd do that in the database? Because I know you can close a bunch
of commits by merging into the destination branch manually and then
pushing all the commits up. That is assuming you have the submit
permission and gerrit automatically closes the changesets for you
right?
Thanks for all the advice though, there definitely seems to be
something workable in all the options you mentioned above.
Antony Stubbs
> On Jul 20, 11:56 pm, Shawn Pearce <s...@google.com> wrote:
>> git rebase -i. Its not that hard these days. Its a bit annoying.
>> And sometimes it just isn't worth it.
>>
>> For example in JGit recently we have a series of over 100 commits
>> being developed, with a lot of code refactoring going on. If an issue
>> was identified during review in say the 5th commit in the series,
>> fixing it just isn't worth it because the entire section of code might
>> be gone by the 30th commit. But we're mostly talking about debatable
>> style issues here ("this method should be purple not blue") and not
>> logic correctness ("this should be <= not <").
>>
>>> And you generally
>>> have to approve the changeset and then get them to fix their changes
>>> in another changeset. Or you abandon all the changesets, get them to
>>> squash all the commits together and approve that one.
>>
>> I try to avoid doing this. I force them to learn how to use git
>> rebase -i to go back and fix their change.
>
> So in this JGit example if there was a logic problem in the 5th
> commit, you would get the author to rebase all the 100 commits? And
> then you'd abandon the 100 previous changesets and just approve the
> new mega commit?
They still stay as individual changes. He's talking about rebasing, not
squashing.
> Obviously abandoning 100 changesets would be annoying, so I guess
> you'd do that in the database? Because I know you can close a bunch
> of commits by merging into the destination branch manually and then
> pushing all the commits up. That is assuming you have the submit
> permission and gerrit automatically closes the changesets for you
> right?
I would imagine they rebase the commits, but they still have the
same changeId, so Gerrit recognises then as just new versions of
the same change.
Cheers,
Antony Stubbs,
Joel
> They still stay as individual changes. He's talking about rebasing, not
> squashing.
squash too. I just tried editing a commit that was 4 away from the
HEAD with git rebase -i and then changed pick to edit.
amended the change and did git rebase --continue and it worked great.
I didn't know you could do this, this is a lifesaver :-)
> I would imagine they rebase the commits, but they still have the
> same changeId, so Gerrit recognises then as just new versions of
> the same change.
keep amending 1 commit on their topic branch until their change is
merged, because editing previous commits was hard if not impossible.
Turns out that I was wrong which is awesome. This will definitely
make life easier.
Pat Notz
On Tue, Jul 20, 2010 at 7:56 AM, Shawn Pearce <s...@google.com> wrote:
>
> Within Google (and at least one other company using Gerrit) we have
> recently setup a bleeding edge master version of Gerrit Code Review
> and are using the personal sandbox branch concept:
>
> Push Branch +3 refs/sandbox/${username}/*
> Code Review +2 refs/sandbox/${username}/*
> Submit +1 refs/sandbox/${username]/*
>
> This gives each developer their own area to work in. But its
> primarily meant for backing up your laptop or your workstation
> overnight, or to help you move a patch between the two before its
> ready to get sent to a reviewer. We aren't really using it as an area
> for peer collaboration.
>
I'm considering doing something similar. Do you still like this idea
after a month or so of usage?
Also, if you wanted to support collaboration on these branches, would
it make sense to allow others to give others upload permission like
this:
Read Access +2 refs/sandbox/*
and, possibly,
Code Review +2 refs/sandbox/*
?
With these two additions it seems like users would still maintain
control of their branches but could collaborate with other devs.
Thanks ~ Pat
Shawn Pearce
> On Tue, Jul 20, 2010 at 7:56 AM, Shawn Pearce <s...@google.com> wrote:
>>
>> Within Google (and at least one other company using Gerrit) we have
>> recently setup a bleeding edge master version of Gerrit Code Review
>> and are using the personal sandbox branch concept:
>>
>> Push Branch +3 refs/sandbox/${username}/*
>> Code Review +2 refs/sandbox/${username}/*
>> Submit +1 refs/sandbox/${username]/*
>>
>> This gives each developer their own area to work in. But its
>> primarily meant for backing up your laptop or your workstation
>> overnight, or to help you move a patch between the two before its
>> ready to get sent to a reviewer. We aren't really using it as an area
>> for peer collaboration.
>>
>
> I'm considering doing something similar. Do you still like this idea
> after a month or so of usage?
I don't know. I don't use them. A quick survey of our internal
server shows 1 sandbox branch. 1. :-\
> Also, if you wanted to support collaboration on these branches, would
> it make sense to allow others to give others upload permission like
> this:
>
> Read Access +2 refs/sandbox/*
>
> and, possibly,
>
> Code Review +2 refs/sandbox/*
>
> ?
>
> With these two additions it seems like users would still maintain
> control of their branches but could collaborate with other devs.
Yes, that does make sense.