{"thread":{"id":"24395","subject":"help with distributed workflow/signoff","startedAt":"2010-07-14T16:33:30Z","lastAt":"2010-07-15T17:42:38Z","messageCount":9,"participants":["Brock Peabody","Jonathan Nieder","Avery Pennarun","Ævar Arnfjörð Bjarmason","A Large Angry SCM"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"145555","messageId":"loom.20100714T180615-173@post.gmane.org","threadId":"24395","inReplyTo":null,"subject":"help with distributed workflow/signoff","fromName":"Brock Peabody","fromEmail":"brock.peabody@gmail.com","sentAt":"2010-07-14T16:33:30Z","receivedAt":"2010-07-14T16:33:30Z","isPatch":false,"sender":{"key":"brock.peabody@gmail.com","avatar":null},"body":"Hi,\nI'm new to the list and have a question about signoffs, and the details of how\ndistributed workflows work in the wild.\n\nWe are looking at converting a large group of developers from using svn to git.\n Our current svn workflow relies on external tools and commit hooks to present\n(poorly) a somewhat distributed model that would be natural with git. \nUnfortunately, we have not been able to connect a few of the dots that we need\nfor our git based workflow. We have a basic idea, loosely based on workflows\npresented in online sources and the \"Pro Git\" book:\n\n Developer -> GateKeeper -> Master Repository\n\nDevelopers wishing to contribute code would push revisions to the gatekeepers'\nrepos, who after reviewing the revisions would push them to the Master\nrepository, from which developers would have only pull access. The GateKeeper\nrepositories would be well known, and observers could easily see which\nGateKeepers had which revisions in their review queues.\n\nThis works fine, except for the fact that there is no trail in the master\nrepository indicating which GateKeeper approved which revision.\n\nWe've scoured the internet for solutions. One idea we have tried and abandoned\nis using 'git commit -s --amend'. This does create a new revision with the\nsignoff information we want, but the problem is that it creates a new revision.\nEventually, the developers (and the GateKeeper repository) will end up with both\nthe revision containing the signoff and one without it.\n\nI've seen evidence of what we're after - by browsing the repository of git\nitself. Each revision in it contains an \"Author\" field and a (frequently\ndifferent) \"Committer\" field. Furthermore, there are often multiple\n\"Signed-off-by\" and \"Acked by\" logs. How are these fields populated? Are new\nrevisions created each time the sign-off information is added? If so, how do the\ncontributors deal with these new revisions when synchronizing with the master\nlater? How are the pre-signoff revisions purged from the bare GateKeeper\nrepositories?\n\nI would greatly welcome and advice on how these workflows are implemented.\n\nThanks in advance,\nBrock Peabody\n"},{"id":"145563","messageId":"20100714171614.GA2799@burratino","threadId":"24395","inReplyTo":"loom.20100714T180615-173@post.gmane.org","subject":"Re: help with distributed workflow/signoff","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-14T17:16:14Z","receivedAt":"2010-07-14T17:16:14Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Brock,\n\nBrock Peabody wrote:\n\n> Developers wishing to contribute code would push revisions to the gatekeepers'\n> repos, who after reviewing the revisions would push them to the Master\n> repository, from which developers would have only pull access.\n[...]\n> This works fine, except for the fact that there is no trail in the master\n> repository indicating which GateKeeper approved which revision.\n[...]\n> I've seen evidence of what we're after - by browsing the repository of git\n> itself. Each revision in it contains an \"Author\" field and a (frequently\n> different) \"Committer\" field. Furthermore, there are often multiple\n> \"Signed-off-by\" and \"Acked by\" logs. How are these fields populated?\n\nIn the Linux kernel and similar projects, there is a sharp distinction\nbetween patch authors and committers[1]. The life cycle of a patch is\nsomething like this:\n\nA patch is written and a rough version is mailed to interested parties.\n‘git format-patch’ or ‘git request-pull’ can be useful for this.\n\nThe patch is discussed. Subsystem maintainers, reviewers, testers,\nand others try out the patch and form an opinion on whether it is\n“cooked” or needs more improvements. Some of these people (including\nthe original author) might suggest improved versions of the patch;\neach person doing so adds her name to the bottom of the Signed-off-by\nlist to indicate that to the best of her knowledge the updated patch\nis not proprietary but is suitable for inclusion in the project.\n\nAt a certain point, the patch is ready. A committer applies it,\nadding appropriate Acked-by etc headers to summarize the discussion\nfor reference when the code breaks :). ‘git am -s’ can take care of\nadding the sign-off in this case.\n\nThe trajectory of the patch afterwards can be tracked through merges.\nThe commit passes through a hierarchy of maintainers, usually with\nthe help of ‘git request-pull’. There is no need to recertify\norigin at this point, since the commit object is already set in\nstone.\n\n-----\n\nWait a second. The patch is never in a git tree until it is fully\ncooked?\n\nWell, no, as long as it is made clear, it is perfectly fine to publish\na patch as a commit before it is cooked. But the relevant git\nbranches at this point are just a distribution medium, and those\ncommits are not meant to be pulled by anyone upstream. Once the\npatches seem cooked, a person can _propose_ a branch to be pulled. If\nthat branch is completely broken, it is back to the drawing board, and\na new branch is made, usually with the help of ‘git rebase\n--interactive’, or ‘git reset’ followed by ‘git add --patch’.\n\n-----\n\nWhy not keep the false starts? Why such concern with history?\n\nTo get maximum utility out of ‘git bisect’, it is best if each commit\nbuilds cleanly and the result is not too broken.\n\nTo get maximum utility out of ‘git log --grep’ and ‘git log -S’, it\nis best if the commit log clearly and logically explains the design\nof the current code.\n\nProbably only some of these concerns will apply in your case, but\nI thought I should explain the story as well as I could.\n\nHope that helps,\nJonathan\n\n[1] I am thinking in particular of this message as I write:\nhttp://thread.gmane.org/gmane.comp.video.dri.devel/34739/focus=34744\n"},{"id":"145565","messageId":"AANLkTimdVvKYPQn84IQvk7yxMo-gtrjWWqzN-ypXV78X@mail.gmail.com","threadId":"24395","inReplyTo":"loom.20100714T180615-173@post.gmane.org","subject":"Re: help with distributed workflow/signoff","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2010-07-14T17:24:22Z","receivedAt":"2010-07-14T17:24:22Z","isPatch":false,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Wed, Jul 14, 2010 at 12:33 PM, Brock Peabody wrote:\n> We've scoured the internet for solutions.  One idea we have tried and abandoned\n> is using 'git commit -s --amend'. This does create a new revision with the\n> signoff information we want, but the problem is that it creates a new revision.\n> Eventually, the developers (and the GateKeeper repository) will end up with both\n> the revision containing the signoff and one without it.\n\nOne option is to avoid trying to signoff each *revision*, but instead\nsignoff on an entire *batch* of changes. Basically, the maintainer\nwould do something like this:\n\n git checkout master\n git merge --no-ff whateveruser/whateverchanges\n git commit --amend --signoff\n git push\n\nThen the maintainer will have a merge commit that shows he looked at\nit. All the individual sub-patches will remain identical, so nobody's\nhistory gets confused.\n\n> I've seen evidence of what we're after - by browsing the repository of git\n> itself.  Each revision in it contains an \"Author\" field and a (frequently\n> different) \"Committer\" field.  Furthermore, there are often multiple\n> \"Signed-off-by\" and \"Acked by\" logs.  How are these fields populated?  Are new\n> revisions created each time the sign-off information is added? If so, how do the\n> contributors deal with these new revisions when synchronizing with the master\n> later?  How are the pre-signoff revisions purged from the bare GateKeeper\n> repositories?\n\nThe way it works in git is that patches are sent in via this mailing\nlist - the maintainer rarely uses git merge or pull (except to merge\nhis own branches). Thus, the \"author\" is the person who emailed the\npatch, and the \"committer\" is the person who applied the patch to his\nown tree.\n\nThe result is an elegant looking history, but of course, the committer\ngenerates an entirely different commit id than the author had in his\nown history. Since the maintainer never pulls directly from the\nauthor, it's each author's job to wipe/rebase his own history\noccasionally to stay in sync with the \"upstream.\"\n\nFor an open source project, where most contributions are by volunteers\nand need to have their patches reviewed multiple times before\nsubmission - and frequently, more patchsets are rejected than applied\n- this works reasonably well. For a company where (in my experience\nat least) most people's patches *are* applied, and the ratio of\nreviewers to coders is much lower, that's much less workable. And\nunfortunately the elegant looking multiple-signed-off-by or acked-by\nlines don't work so well for that.\n\nOh, now that I think of it, you might find git-notes useful. I've\nnever used it but I understand it lets you add lines to the log\nmessages retroactively. Of course, that can be both a blessing and a\ncurse. If you can retroactively change signoffs, the signoffs aren't\nthat valuable.\n\nHave fun,\n\nAvery\n"},{"id":"145571","messageId":"loom.20100714T193832-914@post.gmane.org","threadId":"24395","inReplyTo":"20100714171614.GA2799@burratino","subject":"Re: help with distributed workflow/signoff","fromName":"Brock Peabody","fromEmail":"brock.peabody@gmail.com","sentAt":"2010-07-14T17:49:42Z","receivedAt":"2010-07-14T17:49:42Z","isPatch":false,"sender":{"key":"brock.peabody@gmail.com","avatar":null},"body":"Hi Jonathan,\n\nThanks for your quick, informative response. This is the information I was\nhoping to get - how the tool is used by its primary audience. It's going to\ntake me a while to fully absorb it, but I suspect (as Avery points out in his\nreply) that this type of workflow is less workable when there are fewer\nreviewers and more patches, most of which are accepted as-is.\n\nNevertheless, I want to make sure that we keep in mind the way the tool is meant\nto be used when choosing our new workflow, rather than trying to force it into a\nmold inspired by svn. It would probably do us no harm to be forced to have\nfewer, more well considered patches.\n\nCheers,\nBrock\n"},{"id":"145573","messageId":"loom.20100714T195109-665@post.gmane.org","threadId":"24395","inReplyTo":"AANLkTimdVvKYPQn84IQvk7yxMo-gtrjWWqzN-ypXV78X@mail.gmail.com","subject":"Re: help with distributed workflow/signoff","fromName":"Brock Peabody","fromEmail":"brock.peabody@gmail.com","sentAt":"2010-07-14T18:06:28Z","receivedAt":"2010-07-14T18:06:28Z","isPatch":false,"sender":{"key":"brock.peabody@gmail.com","avatar":null},"body":"Hi Avery,\n\nAvery Pennarun gmail.com> writes:\n\n> For an open source project, where most contributions are by volunteers\n> and need to have their patches reviewed multiple times before\n> submission - and frequently, more patchsets are rejected than applied\n> - this works reasonably well. For a company where (in my experience\n> at least) most people's patches *are* applied, and the ratio of\n> reviewers to coders is much lower, that's much less workable. And\n> unfortunately the elegant looking multiple-signed-off-by or acked-by\n> lines don't work so well for that.\n\nI think you've hit the nail on the head here. In our environment, commits are\nfrequent and signoffs prompt. Revisions are very rarely rejected, and will\nnever pass through more than one reviewer except in extreme cases. Contributors\nwill have little tolerance for per-commit time or complexity overhead incurred\nfrom the process.\n\n> Oh, now that I think of it, you might find git-notes useful. I've\n> never used it but I understand it lets you add lines to the log\n> messages retroactively. Of course, that can be both a blessing and a\n> curse. If you can retroactively change signoffs, the signoffs aren't\n> that valuable.\n\nActually, that might be exactly what we need. I'm sure we could set it up in\nsuch a way so that this signoff is only added mechanically, once, when a\nreviewer decides to add a revision to the 'master' repository.\n\nMuch thanks,\nBrock\n"},{"id":"145582","messageId":"AANLkTilo4TnMQcr7p1dfAeQ4tESHjO4Nbr8274hqxOnD@mail.gmail.com","threadId":"24395","inReplyTo":"loom.20100714T195109-665@post.gmane.org","subject":"Re: help with distributed workflow/signoff","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-14T21:03:25Z","receivedAt":"2010-07-14T21:03:25Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Jul 14, 2010 at 18:06, Brock Peabody wrote:\n> Hi Avery,\n>\n> Avery Pennarun gmail.com> writes:\n>\n>> For an open source project, where most contributions are by volunteers\n>> and need to have their patches reviewed multiple times before\n>> submission - and frequently, more patchsets are rejected than applied\n>> - this works reasonably well.  For a company where (in my experience\n>> at least) most people's patches *are* applied, and the ratio of\n>> reviewers to coders is much lower, that's much less workable.  And\n>> unfortunately the elegant looking multiple-signed-off-by or acked-by\n>> lines don't work so well for that.\n>\n> I think you've hit the nail on the head here.  In our environment, commits are\n> frequent and signoffs prompt.  Revisions are very rarely rejected, and will\n> never pass through more than one reviewer except in extreme cases.  Contributors\n> will have little tolerance for per-commit time or complexity overhead incurred\n> from the process.\n\nWell, consider that even if you push most patches through, the peer\nreview you get from having a setup similar to Git's own might very\nwell be worth it. Everyone makes mistakes, having a second set of\neyeballs to look at your code eliminates a lot of that.\n\nThat may not be acceptable to your corporate culture, but consider\nthat most big corporations (e.g. Google) do detailed code review\nbefore anything gets commited to the master repository.\n"},{"id":"145586","messageId":"loom.20100714T232116-579@post.gmane.org","threadId":"24395","inReplyTo":"AANLkTilo4TnMQcr7p1dfAeQ4tESHjO4Nbr8274hqxOnD@mail.gmail.com","subject":"Re: help with distributed workflow/signoff","fromName":"Brock Peabody","fromEmail":"brock.peabody@gmail.com","sentAt":"2010-07-14T21:46:46Z","receivedAt":"2010-07-14T21:46:46Z","isPatch":false,"sender":{"key":"brock.peabody@gmail.com","avatar":null},"body":"Ævar Arnfjörð Bjarmason gmail.com> writes:\n\n> Well, consider that even if you push most patches through, the peer\n> review you get from having a setup similar to Git's own might very\n> well be worth it. Everyone makes mistakes, having a second set of\n> eyeballs to look at your code eliminates a lot of that.\n> \n> That may not be acceptable to your corporate culture, but consider\n> that most big corporations (e.g. Google) do detailed code review\n> before anything gets commited to the master repository.\n\nYes, that's a good point. We are trying to improve our code review processes,\nand I think switching to git is going to help with that down the road. I don't\nwant to make the switch to git and a heavier (if better) process at the same\ntime, however, as it would decrease the chance of either being accepted.\n"},{"id":"145591","messageId":"4C3E462E.9010205@gmail.com","threadId":"24395","inReplyTo":"loom.20100714T232116-579@post.gmane.org","subject":"Re: help with distributed workflow/signoff","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2010-07-14T23:20:14Z","receivedAt":"2010-07-14T23:20:14Z","isPatch":false,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"On 07/14/2010 05:46 PM, Brock Peabody wrote:\n> Ævar Arnfjörð Bjarmason gmail.com> writes:\n>\n>> Well, consider that even if you push most patches through, the peer\n>> review you get from having a setup similar to Git's own might very\n>> well be worth it. Everyone makes mistakes, having a second set of\n>> eyeballs to look at your code eliminates a lot of that.\n>>\n>> That may not be acceptable to your corporate culture, but consider\n>> that most big corporations (e.g. Google) do detailed code review\n>> before anything gets commited to the master repository.\n>\n> Yes, that's a good point. We are trying to improve our code review processes,\n> and I think switching to git is going to help with that down the road. I don't\n> want to make the switch to git and a heavier (if better) process at the same\n> time, however, as it would decrease the chance of either being accepted.\n\nHave you investigated Gerrit Code Review?\n"},{"id":"145641","messageId":"loom.20100715T193756-750@post.gmane.org","threadId":"24395","inReplyTo":"4C3E462E.9010205@gmail.com","subject":"Re: help with distributed workflow/signoff","fromName":"Brock Peabody","fromEmail":"brock.peabody@gmail.com","sentAt":"2010-07-15T17:42:38Z","receivedAt":"2010-07-15T17:42:38Z","isPatch":false,"sender":{"key":"brock.peabody@gmail.com","avatar":null},"body":"A Large Angry SCM gmail.com> writes:\n\n> Have you investigated Gerrit Code Review?\n\nI just did. It looks promising, thanks.\n"}]}