help with distributed workflow/signoff

help with distributed workflow/signoff

[Brock Peabody]

Hi,
I'm new to the list and have a question about signoffs, and the details of how
distributed workflows work in the wild.

We are looking at converting a large group of developers from using svn to git.
 Our current svn workflow relies on external tools and commit hooks to present
(poorly) a somewhat distributed model that would be natural with git. 
Unfortunately, we have not been able to connect a few of the dots that we need
for our git based workflow.  We have a basic idea, loosely based on workflows
presented in online sources and the "Pro Git" book:

  Developer -> GateKeeper -> Master Repository

Developers wishing to contribute code would push revisions to the gatekeepers'
repos, who after reviewing the revisions would push them to the Master
repository, from which developers would have only pull access.  The GateKeeper
repositories would be well known, and observers could easily see which
GateKeepers had which revisions in their review queues.

This works fine, except for the fact that there is no trail in the master
repository indicating which GateKeeper approved which revision.

We've scoured the internet for solutions.  One idea we have tried and abandoned
is using 'git commit -s --amend'. This does create a new revision with the
signoff information we want, but the problem is that it creates a new revision.
Eventually, the developers (and the GateKeeper repository) will end up with both
the revision containing the signoff and one without it.

I've seen evidence of what we're after - by browsing the repository of git
itself.  Each revision in it contains an "Author" field and a (frequently
different) "Committer" field.  Furthermore, there are often multiple
"Signed-off-by" and "Acked by" logs.  How are these fields populated?  Are new
revisions created each time the sign-off information is added? If so, how do the
contributors deal with these new revisions when synchronizing with the master
later?  How are the pre-signoff revisions purged from the bare GateKeeper
repositories?

I would greatly welcome and advice on how these workflows are implemented.

Thanks in advance,
Brock Peabody

Re: help with distributed workflow/signoff

[Jonathan Nieder]

Hi Brock,

Brock Peabody wrote:

> Developers wishing to contribute code would push revisions to the gatekeepers'
> repos, who after reviewing the revisions would push them to the Master
> repository, from which developers would have only pull access.
[...]
> This works fine, except for the fact that there is no trail in the master
> repository indicating which GateKeeper approved which revision.
[...]
> I've seen evidence of what we're after - by browsing the repository of git
> itself.  Each revision in it contains an "Author" field and a (frequently
> different) "Committer" field.  Furthermore, there are often multiple
> "Signed-off-by" and "Acked by" logs.  How are these fields populated?

In the Linux kernel and similar projects, there is a sharp distinction
between patch authors and committers[1].  The life cycle of a patch is
something like this:

A patch is written and a rough version is mailed to interested parties.
‘git format-patch’ or ‘git request-pull’ can be useful for this.

The patch is discussed.  Subsystem maintainers, reviewers, testers,
and others try out the patch and form an opinion on whether it is
“cooked” or needs more improvements.  Some of these people (including
the original author) might suggest improved versions of the patch;
each person doing so adds her name to the bottom of the Signed-off-by
list to indicate that to the best of her knowledge the updated patch
is not proprietary but is suitable for inclusion in the project.

At a certain point, the patch is ready.  A committer applies it,
adding appropriate Acked-by etc headers to summarize the discussion
for reference when the code breaks :).  ‘git am -s’ can take care of
adding the sign-off in this case.

The trajectory of the patch afterwards can be tracked through merges.
The commit passes through a hierarchy of maintainers, usually with
the help of ‘git request-pull’.  There is no need to recertify
origin at this point, since the commit object is already set in
stone.

-----

Wait a second.  The patch is never in a git tree until it is fully
cooked?

Well, no, as long as it is made clear, it is perfectly fine to publish
a patch as a commit before it is cooked.  But the relevant git
branches at this point are just a distribution medium, and those
commits are not meant to be pulled by anyone upstream.  Once the
patches seem cooked, a person can _propose_ a branch to be pulled.  If
that branch is completely broken, it is back to the drawing board, and
a new branch is made, usually with the help of ‘git rebase
--interactive’, or ‘git reset’ followed by ‘git add --patch’.

-----

Why not keep the false starts?  Why such concern with history?

To get maximum utility out of ‘git bisect’, it is best if each commit
builds cleanly and the result is not too broken.

To get maximum utility out of ‘git log --grep’ and ‘git log -S’, it
is best if the commit log clearly and logically explains the design
of the current code.

Probably only some of these concerns will apply in your case, but
I thought I should explain the story as well as I could.

Hope that helps,
Jonathan

[1] I am thinking in particular of this message as I write:
http://thread.gmane.org/gmane.comp.video.dri.devel/34739/focus=34744

Re: help with distributed workflow/signoff

[Avery Pennarun]

On Wed, Jul 14, 2010 at 12:33 PM, Brock Peabody <brock.peabody@gmail.com> wrote:
> We've scoured the internet for solutions.  One idea we have tried and abandoned
> is using 'git commit -s --amend'. This does create a new revision with the
> signoff information we want, but the problem is that it creates a new revision.
> Eventually, the developers (and the GateKeeper repository) will end up with both
> the revision containing the signoff and one without it.

One option is to avoid trying to signoff each *revision*, but instead
signoff on an entire *batch* of changes.  Basically, the maintainer
would do something like this:

   git checkout master
   git merge --no-ff whateveruser/whateverchanges
   git commit --amend --signoff
   git push

Then the maintainer will have a merge commit that shows he looked at
it.  All the individual sub-patches will remain identical, so nobody's
history gets confused.

> I've seen evidence of what we're after - by browsing the repository of git
> itself.  Each revision in it contains an "Author" field and a (frequently
> different) "Committer" field.  Furthermore, there are often multiple
> "Signed-off-by" and "Acked by" logs.  How are these fields populated?  Are new
> revisions created each time the sign-off information is added? If so, how do the
> contributors deal with these new revisions when synchronizing with the master
> later?  How are the pre-signoff revisions purged from the bare GateKeeper
> repositories?

The way it works in git is that patches are sent in via this mailing
list - the maintainer rarely uses git merge or pull (except to merge
his own branches).  Thus, the "author" is the person who emailed the
patch, and the "committer" is the person who applied the patch to his
own tree.

The result is an elegant looking history, but of course, the committer
generates an entirely different commit id than the author had in his
own history.  Since the maintainer never pulls directly from the
author, it's each author's job to wipe/rebase his own history
occasionally to stay in sync with the "upstream."

For an open source project, where most contributions are by volunteers
and need to have their patches reviewed multiple times before
submission - and frequently, more patchsets are rejected than applied
- this works reasonably well.  For a company where (in my experience
at least) most people's patches *are* applied, and the ratio of
reviewers to coders is much lower, that's much less workable.  And
unfortunately the elegant looking multiple-signed-off-by or acked-by
lines don't work so well for that.

Oh, now that I think of it, you might find git-notes useful.  I've
never used it but I understand it lets you add lines to the log
messages retroactively.  Of course, that can be both a blessing and a
curse.  If you can retroactively change signoffs, the signoffs aren't
that valuable.

Have fun,

Avery

Re: help with distributed workflow/signoff

[Brock Peabody]

Hi Jonathan,

Thanks for your quick, informative response.  This is the information I was
hoping to get - how the tool is used by its primary audience.  It's going to
take me a while to fully absorb it, but I suspect (as Avery points out in his
reply) that this type of workflow is less workable when there are fewer
reviewers and more patches, most of which are accepted as-is.

Nevertheless, I want to make sure that we keep in mind the way the tool is meant
to be used when choosing our new workflow, rather than trying to force it into a
mold inspired by svn.  It would probably do us no harm to be forced to have
fewer, more well considered patches.

Cheers,
Brock

Re: help with distributed workflow/signoff

[Brock Peabody]

Hi Avery,

Avery Pennarun <apenwarr <at> gmail.com> writes:

> For an open source project, where most contributions are by volunteers
> and need to have their patches reviewed multiple times before
> submission - and frequently, more patchsets are rejected than applied
> - this works reasonably well.  For a company where (in my experience
> at least) most people's patches *are* applied, and the ratio of
> reviewers to coders is much lower, that's much less workable.  And
> unfortunately the elegant looking multiple-signed-off-by or acked-by
> lines don't work so well for that.

I think you've hit the nail on the head here.  In our environment, commits are
frequent and signoffs prompt.  Revisions are very rarely rejected, and will
never pass through more than one reviewer except in extreme cases.  Contributors
will have little tolerance for per-commit time or complexity overhead incurred
from the process.

> Oh, now that I think of it, you might find git-notes useful.  I've
> never used it but I understand it lets you add lines to the log
> messages retroactively.  Of course, that can be both a blessing and a
> curse.  If you can retroactively change signoffs, the signoffs aren't
> that valuable.

Actually, that might be exactly what we need.  I'm sure we could set it up in
such a way so that this signoff is only added mechanically, once, when a
reviewer decides to add a revision to the 'master' repository.

Much thanks,
Brock

Re: help with distributed workflow/signoff

[Ævar Arnfjörð Bjarmason]

On Wed, Jul 14, 2010 at 18:06, Brock Peabody <brock.peabody@gmail.com> wrote:
> Hi Avery,
>
> Avery Pennarun <apenwarr <at> gmail.com> writes:
>
>> For an open source project, where most contributions are by volunteers
>> and need to have their patches reviewed multiple times before
>> submission - and frequently, more patchsets are rejected than applied
>> - this works reasonably well.  For a company where (in my experience
>> at least) most people's patches *are* applied, and the ratio of
>> reviewers to coders is much lower, that's much less workable.  And
>> unfortunately the elegant looking multiple-signed-off-by or acked-by
>> lines don't work so well for that.
>
> I think you've hit the nail on the head here.  In our environment, commits are
> frequent and signoffs prompt.  Revisions are very rarely rejected, and will
> never pass through more than one reviewer except in extreme cases.  Contributors
> will have little tolerance for per-commit time or complexity overhead incurred
> from the process.

Well, consider that even if you push most patches through, the peer
review you get from having a setup similar to Git's own might very
well be worth it. Everyone makes mistakes, having a second set of
eyeballs to look at your code eliminates a lot of that.

That may not be acceptable to your corporate culture, but consider
that most big corporations (e.g. Google) do detailed code review
before anything gets commited to the master repository.

Re: help with distributed workflow/signoff

[Brock Peabody]

Ævar Arnfjörð Bjarmason <avarab <at> gmail.com> writes:

> Well, consider that even if you push most patches through, the peer
> review you get from having a setup similar to Git's own might very
> well be worth it. Everyone makes mistakes, having a second set of
> eyeballs to look at your code eliminates a lot of that.
> 
> That may not be acceptable to your corporate culture, but consider
> that most big corporations (e.g. Google) do detailed code review
> before anything gets commited to the master repository.

Yes, that's a good point.  We are trying to improve our code review processes,
and I think switching to git is going to help with that down the road.  I don't
want to make the switch to git and a heavier (if better) process at the same
time, however, as it would decrease the chance of either being accepted.

Re: help with distributed workflow/signoff

[A Large Angry SCM]

On 07/14/2010 05:46 PM, Brock Peabody wrote:
> Ævar Arnfjörð Bjarmason<avarab<at>  gmail.com>  writes:
>
>> Well, consider that even if you push most patches through, the peer
>> review you get from having a setup similar to Git's own might very
>> well be worth it. Everyone makes mistakes, having a second set of
>> eyeballs to look at your code eliminates a lot of that.
>>
>> That may not be acceptable to your corporate culture, but consider
>> that most big corporations (e.g. Google) do detailed code review
>> before anything gets commited to the master repository.
>
> Yes, that's a good point.  We are trying to improve our code review processes,
> and I think switching to git is going to help with that down the road.  I don't
> want to make the switch to git and a heavier (if better) process at the same
> time, however, as it would decrease the chance of either being accepted.

Have you investigated Gerrit Code Review?

Re: help with distributed workflow/signoff

[Brock Peabody]

A Large Angry SCM <gitzilla <at> gmail.com> writes:

> Have you investigated Gerrit Code Review?

I just did. It looks promising, thanks.