<div dir="ltr"><div>On Mon, Jun 3, 2013 at 3:59 PM, Glyph <span dir="ltr"><<a href="mailto:glyph@twistedmatrix.com" target="_blank">glyph@twistedmatrix.com</a>></span> wrote:<br></div><div><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br></div><div>One suggestion that almost everybody made immediately was: we should use Github for code reviews.</div>
</div></blockquote><div> </div><div>I'm +1 on the whole proposition as described.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><div>Finally, my own minor concern: Github has no notion of a "code review" as a unit of work.  A pull request is just "open" until it is "closed".  Closing pull requests to request changes would be jarring to the cultural norms associated with Github's UI.  All the github users I've spoken with, even those who follow processes which are effectively identical to Twisted's, have assured me that this is not really an issue.  A code review is "accepted" when you merge it; it's "rejected" if the pull request is still open but has some comments on it.  This will make porting over <<a href="http://twistedmatrix.com/highscores/" target="_blank">http://twistedmatrix.com/highscores/</a>> a bit challenging, but I think it would be worth letting that break for the time being.</div>
</div></div></blockquote><div><br></div><div>I don't really like the idea of maintaining review state in Trac, especially since one of the points of this discussion is to make the life of the reviewer easier. My feeling is that the slight difference in review workflow on PRs -- the fact that there is no "responsibility transfer" mechanism -- will not be a serious problem in practice, and that we should have a culture of closing abandoned PRs.<br>
</div><div><br></div></div>-- <br>Christopher Armstrong<br><a href="http://radix.twistedmatrix.com/">http://radix.twistedmatrix.com/</a><br><a href="http://planet-if.com/">http://planet-if.com/</a><br><br>
</div></div></div>