help with distributed workflow/signoff

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

Hi Brock,

Brock Peabody wrote:

[...]

[...]

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

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

On Wed, Jul 14, 2010 at 12:33 PM, Brock Peabody <brock.peabody@gmail.com> wrote:

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.

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

Hi Avery,

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

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.

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

On Wed, Jul 14, 2010 at 18:06, Brock Peabody <brock.peabody@gmail.com> wrote:

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.

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

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.

On 07/14/2010 05:46 PM, Brock Peabody wrote:

Have you investigated Gerrit Code Review?

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

I just did. It looks promising, thanks.