Shared topic branches: how to collaborate
Greg Hurrell
We're just starting to use Gerrit internally at causes.com and are
having some trouble figuring out the best workflow to permit
collaboration on topic branches.
(1) We do a lot of pair programming, often swapping the "driving", so
on a given topic branch we might have a couple commits by one person
and a few by another. We find that when it comes time to submit this
for review (or when amending in response to review comments) that
Gerrit rejects any push to refs/for/master which contains a commit
with an author other than the person pushing.
From reading the group and the docs etc it seems that this is behaving
as intended. The docs and our experiments suggest that even granting
"Forge Author Identity" is not sufficient for this, and that
permission is really intended for another use case. So, what are
people doing to handle this need (shared authorship and ownership of a
series of commits)?
One partial answer looks to be creating a merge commit and pushing
that for review instead. One problem with that is the resulting diff
isn't very reviewable (ie. there is no diff). Another is that it still
doesn't permit rebasing of the collaborator's commits. Without
rebasing, even if you can apply a fixup commit later on in the branch
for an earlier mistake, if the earlier commit somehow breaks the build
then you're harming the bisectability of your history.
Does that mean that squashing the topic and amending with --author is
the only way?
(2) The other problem we've found is that we can't really use the
Gerrit repo as a temporary store for works in progress that aren't
ready for review yet. For example, if we make an unrestricted
namespace that people can push into (eg. refs/heads/topics) to share
their works in progress with collaborators, then once the topic is
baked and is ready for submission it can't be submitted to refs/for/
master because Gerrit has already seen the Change-Ids and considers
that to already have been merged.
Options we've considered are rewriting history to assign new Change-
Ids, or patching the hook responsible for inserting the Change-Ids to
remove/omit them for any commit pushed to the refs/heads/topics
namespace.
Another that we considered is just pushing directly to refs/for/
master, but somehow marking the commit as not-yet-ready for review
(eg. by pushing to refs/for/master/wip/my-topic). When ready, amend
and push, omitting the "wip" from the refname. Reviewers can filter
out the noise of wip commits with a filter like "-topic:^.*wip.*
Does anybody have any better ideas? We don't really want to introduce
another remote just for temporarily storing work in progress.
-Greg
Jay Soffian
> Hi,
>
> We're just starting to use Gerrit internally at causes.com and are
> having some trouble figuring out the best workflow to permit
> collaboration on topic branches.
>
> (1) We do a lot of pair programming, often swapping the "driving", so
> on a given topic branch we might have a couple commits by one person
> and a few by another. We find that when it comes time to submit this
> for review (or when amending in response to review comments) that
> Gerrit rejects any push to refs/for/master which contains a commit
> with an author other than the person pushing.
>
> From reading the group and the docs etc it seems that this is behaving
> as intended. The docs and our experiments suggest that even granting
> "Forge Author Identity" is not sufficient for this, and that
> permission is really intended for another use case. So, what are
> people doing to handle this need (shared authorship and ownership of a
> series of commits)?
Why not create a topic branch, and then work off that topic.
In gerrit, create new branch "topic" from "master". Then:
devA$ git fetch
devA$ git checkout -b topic origin/topic
devA$ edit, commit, git push origin HEAD:refs/for/topic
review and submit change to topic. Next:
devB$ git fetch
devB$ git checkout -b topic/origin topic
devB$ edit, commit, git push origin HEAD:refs/for/topic
Rinse, wash, repeat. When topic is all done, then it gets merged to
master by one of the devs:
devA$ git fetch
devA$ git checkout origin/master
devA$ git merge origin/topic
Review the merge commit, which sadly needs to be done outside of gerrit. Then:
devA$ git push origin HEAD:master
You can also push the merge commit to refs/for/master, but since
gerrit doesn't really help in reviewing merge commits, there's not a
lot of benefit in doing so.
> (2) The other problem we've found is that we can't really use the
> Gerrit repo as a temporary store for works in progress that aren't
> ready for review yet. For example, if we make an unrestricted
> namespace that people can push into (eg. refs/heads/topics) to share
> their works in progress with collaborators, then once the topic is
> baked and is ready for submission it can't be submitted to refs/for/
> master because Gerrit has already seen the Change-Ids and considers
> that to already have been merged.
I think the problem is that you're using refs/heads. See using a
sandbox as described here:
http://gerrit-documentation.googlecode.com/svn/Documentation/2.1.7/access-control.html
Also http://www.mailinglistarchive.com/html/repo-d...@googlegroups.com/2010-07/msg00085.html
j.
Greg Hurrell
Gerrit and have now been able to iron out most of the kinks and pain
points.
On Oct 6, 9:20 am, Jay Soffian <jaysoff...@gmail.com> wrote:
>
> Why not create a topic branch, and then work off that topic.
problem of Gerrit thinking there are "no new changes". Especially when
combined with the "Cherry Pick" integration option, it actually brings
us quite close to our old workflow (which was to use topic branches,
rebase them, and then merge in using --no-ff after they'd been
reviewed via Rietveld or directly on GitHub).
We've also found that this is not purely a technical/configuration
issue, but also a "wetware"/human one. Namely, there are two practices
we can adopt that reduce the overhead and noise of developing topic
branches directly in Gerrit:
- pushing commits to Gerrit only when they're ready for review; never
using Gerrit as a temporary scratch space for works in progress
- explicitly picking reviewers rather than just dumping stuff into the
global queue expecting someone to pick it up
> I think the problem is that you're using refs/heads. See using a
> sandbox as described here:
main problem seemed to be that I had added the "Forge Author Identity"
permission on "refs/for/refs/*" rather than on "refs/*". With it on
"refs/*", developers can pair on a topic branch, rebase each other's
commits, and push to "refs/for/*" without problems.
> http://gerrit-documentation.googlecode.com/svn/Documentation/2.1.7/ac...
>
> Also http://www.mailinglistarchive.com/html/repo-d...@googlegroups.com/...
Thanks for the links!
The only remaining issue we still need to face is how to push a topic
through, bypassing review, so that we can run the code on our remote
staging servers during development, prior to review. (We have a very
large data set, so when we want to test out an idea against real data,
the staging environment is good for that as it has a regularly
refreshed copy of our production data.)
For now, the simplest solution seems to be squashing the topic branch
and suppressing the Change-Id, so that Gerrit won't later claim there
are "no new changes", and letting the commit go through to a sandbox
area.
Greg
Magnus Bäck
Greg Hurrell <greg.h...@causes.com> wrote:
[...]
> The only remaining issue we still need to face is how to push a topic
> through, bypassing review, so that we can run the code on our remote
> staging servers during development, prior to review. (We have a very
> large data set, so when we want to test out an idea against real data,
> the staging environment is good for that as it has a regularly
> refreshed copy of our production data.)
>
> For now, the simplest solution seems to be squashing the topic branch
> and suppressing the Change-Id, so that Gerrit won't later claim there
> are "no new changes", and letting the commit go through to a sandbox
> area.
Why not run the tests against the uploaded (and unmerged) changes? Just
set up the staging servers to fetch and check out refs/changes/X/Y and
you're all set.
--
Magnus Bäck Opinions are my own and do not necessarily
SW Configuration Manager represent the ones of my employer, etc.
Sony Ericsson
Greg Hurrell
> On Wednesday, October 19, 2011 at 06:47 CEST,
>
> [...]
>
> > The only remaining issue we still need to face is how to push a topic
> > through, bypassing review, so that we can run the code on our remote
> > staging servers during development, prior to review. (We have a very
> > large data set, so when we want to test out an idea against real data,
> > the staging environment is good for that as it has a regularly
> > refreshed copy of our production data.)
>
> > For now, the simplest solution seems to be squashing the topic branch
> > and suppressing the Change-Id, so that Gerrit won't later claim there
> > are "no new changes", and letting the commit go through to a sandbox
> > area.
>
> Why not run the tests against the uploaded (and unmerged) changes? Just
> set up the staging servers to fetch and check out refs/changes/X/Y and
> you're all set.
replicating to GitHub and our deploy process continues to pull from
GitHub as the sole authoritative repo.
-Greg
Magnus Bäck
Jonas Bang <em...@jonasbang.dk> wrote:
> We have been struggling with the exact same 2 issues. Here is how we
> have solved it / worked around it.
>
> (1)
> We have solved this by enabling "Forge Author". This enables the last
> "driver" to push all commits on the feature branch for review even he
> is not author of all of them. Before pushing for review the "driver"
> rebases the feature branch to get rid of all merge commits, as it is
> not allowed to push merge commits. While working on the feature branch
> we push directly to refs/heads/feature/<branch>, i.e. no review, the
> review is done when pushing the feature branch to refs/for/develop.
> As you can see we have looked at Git Flow for branch structure.
>
> So, in short, while working on the feature branch you merge with the
> remote feature branch to get other developers feature commits, and you
> merge with origin/develop to get latest merged commits, and you push
> directly to refs/heads/feature/<branch>.
Why not rebase instead of merge, and get rid of the useless merge
commits?
> Once the feature is done, you rebase on origin/develop (using -f is
> advised) to get rid of merge commits, and push to refs/for/develop.
> If any commits get rejected it is the job of the "driver" to fix and
> amend, even commits he did not author, so it might require some pair
> coding. Once all commits are reviewed and merged, the local and remote
> feature branch are obsolete and must be deleted.
So you review each commit, one by one, after they're rebased onto
'develop'? Why not do that review up front before the commits hit
the feature branch? What's the gain in postponing the review?
[...]
--
Magnus Bäck
ba...@google.com