Glyph
2013-06-03 20:59:42 UTC
Hi Twisted developers,
This weekend I had a discussion with many Twisted developers, both local to and visiting San Francisco. The topic came up of how to get more long-term contributors to participate more regularly in the project - particularly, doing code reviews, but also, developing and contributing to complex fixes and features that new contributors might not be able to tackle.
One suggestion that almost everybody made immediately was: we should use Github for code reviews.
In the past, I've heard this suggestion given mainly as a way to contribute more code, which does not appeal to me, since we are already swamped reviewing all the code that is currently being contributed.
This time, however, it's been pitched as a way to get people to do more reviews, which I'm keenly interested in. Why would people do more reviews on Github? In a nutshell, it's a lot less work. Here are some reasons why:
Instead of having to run 'force-builds' on the command line, or load a buildbot status page, Github has a way for a build system to report build success automatically, so you can see immediately within a pull request if the changes that it proposes are "good to merge". You can see this at work with Travis here: <https://github.com/twisted/klein/pull/11>. Originally I thought that this was a Travis-CI feature, but have since learned that this is apparently easy - trivial even - to hook up to Buildbot, since it's a simple HTTP API to invoke when a build completes, and there is even some existing buildbot infrastructure (deployed by Django, among others) to automate it.
Instead of having to describe each patch location so that you can comment on it in a single message, if you want to put a comment on a particular part of a diff in a Github pull request, you can just click on it and start typing.
In addition to the diff, it's reasonably easy to see the code in context on the web, which is faster than getting it into one's local development environment.
If a review is successful, instead of having to have a local development environment, a committer can just hit the "approve" button and it's landed immediately.
Instead of having to read through all history ever to see what's still relevant, a pull request will hide comments that address outdated diffs, allowing the change author to easily see what remains.
These advantages are not comprehensive, but they're the more significant ones I remember from this discussion.
A prerequisite for using Github for code reviews would be using Git rather than Subversion. Luckily there's not much work to do in this area, thanks to Tom's excellent work on the Git import and automatic Github mirror. As a bonus, by using Git instead of Subversion, we can start properly recording merge metadata.
In this discussion, Alex Gaynor pointed out that Django has a hybrid workflow where they still use Trac for bug tracking, and Github for code review. We would therefore not need to come up with a way to migrate all of our tickets to Github issues (which seems, oddly, to be fairly unpopular even among those who like github a lot).
What would need to happen in order for this to take place?
We'd need some consensus (hence this message).
We'd need to update the release process <http://twistedmatrix.com/trac/wiki/ReleaseProcess> and our development documentation <http://twistedmatrix.com/trac/wiki/BasicGuideToContributingCode> to refer to the relevant Git commands rather than Subversion commands.
We'd need a redirect from <http://twistedmatrix.com/trac/browser/> and <http://twistedmatrix.com/trac/changeset/> that would point at <https://github.com/twisted/twisted> and <https://github.com/twisted/twisted/commits/> respectively.
We'd need a Github web hook that could poke Buildbot to kick off commits.
We'd need Buildbot integration to update Github pull requests with build results when builds complete.
We'd need someone to install git rather than bzr on all the buildbots, and update the configuration of the builders to get the code from a git rather than Subversion URL.
Someone will need to convert every open ticket in review to a pull request.
I do anticipate some objections.
One objection is that each of the above tasks is going to take some work.
I am fairly confident that some of the people who have educated me here will step forward to volunteer to do it. Please reply to this message if you'd like to volunteer, saying what you'd like to volunteer to do. If not, then I guess that objection stands :-).
Another is that this might not be worth that investment of effort. This is why it was nice to have Alex contributing to the discussion: Django did basically this very change (right down to the "Trac for tickets / Github for pull requests" distinction), at a much higher scale than we have, and as he described it the change was *well* worth it.
Another objection is that Github is proprietary software, and an externally-maintained service that we'd be depending upon.
One solution to the "proprietary software" thing is the availability of the MIT-licensed <http://gitlab.org>. It's a largely feature-complete clone of Github; if, for some reason, we need to migrate away from Github in a hurry, it will be relatively painless to set up Gitlab instead, and the fact that Git is a DVCS means every contributor will serve as a backup. The main reason I would not suggest just deploying it is that it creates another sticky infrastructure-management problem, and while Braid is great, I'd prefer to avoid creating more work in that area. Github also has APIs for literally all of their features, so we can create a backup script.
(Also worth noting: Gitlab is an open-source competitor to Github, but they still trust Github enough to <https://github.com/gitlabhq/> host their own development there.)
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 <http://twistedmatrix.com/highscores/> a bit challenging, but I think it would be worth letting that break for the time being.
-glyph
This weekend I had a discussion with many Twisted developers, both local to and visiting San Francisco. The topic came up of how to get more long-term contributors to participate more regularly in the project - particularly, doing code reviews, but also, developing and contributing to complex fixes and features that new contributors might not be able to tackle.
One suggestion that almost everybody made immediately was: we should use Github for code reviews.
In the past, I've heard this suggestion given mainly as a way to contribute more code, which does not appeal to me, since we are already swamped reviewing all the code that is currently being contributed.
This time, however, it's been pitched as a way to get people to do more reviews, which I'm keenly interested in. Why would people do more reviews on Github? In a nutshell, it's a lot less work. Here are some reasons why:
Instead of having to run 'force-builds' on the command line, or load a buildbot status page, Github has a way for a build system to report build success automatically, so you can see immediately within a pull request if the changes that it proposes are "good to merge". You can see this at work with Travis here: <https://github.com/twisted/klein/pull/11>. Originally I thought that this was a Travis-CI feature, but have since learned that this is apparently easy - trivial even - to hook up to Buildbot, since it's a simple HTTP API to invoke when a build completes, and there is even some existing buildbot infrastructure (deployed by Django, among others) to automate it.
Instead of having to describe each patch location so that you can comment on it in a single message, if you want to put a comment on a particular part of a diff in a Github pull request, you can just click on it and start typing.
In addition to the diff, it's reasonably easy to see the code in context on the web, which is faster than getting it into one's local development environment.
If a review is successful, instead of having to have a local development environment, a committer can just hit the "approve" button and it's landed immediately.
Instead of having to read through all history ever to see what's still relevant, a pull request will hide comments that address outdated diffs, allowing the change author to easily see what remains.
These advantages are not comprehensive, but they're the more significant ones I remember from this discussion.
A prerequisite for using Github for code reviews would be using Git rather than Subversion. Luckily there's not much work to do in this area, thanks to Tom's excellent work on the Git import and automatic Github mirror. As a bonus, by using Git instead of Subversion, we can start properly recording merge metadata.
In this discussion, Alex Gaynor pointed out that Django has a hybrid workflow where they still use Trac for bug tracking, and Github for code review. We would therefore not need to come up with a way to migrate all of our tickets to Github issues (which seems, oddly, to be fairly unpopular even among those who like github a lot).
What would need to happen in order for this to take place?
We'd need some consensus (hence this message).
We'd need to update the release process <http://twistedmatrix.com/trac/wiki/ReleaseProcess> and our development documentation <http://twistedmatrix.com/trac/wiki/BasicGuideToContributingCode> to refer to the relevant Git commands rather than Subversion commands.
We'd need a redirect from <http://twistedmatrix.com/trac/browser/> and <http://twistedmatrix.com/trac/changeset/> that would point at <https://github.com/twisted/twisted> and <https://github.com/twisted/twisted/commits/> respectively.
We'd need a Github web hook that could poke Buildbot to kick off commits.
We'd need Buildbot integration to update Github pull requests with build results when builds complete.
We'd need someone to install git rather than bzr on all the buildbots, and update the configuration of the builders to get the code from a git rather than Subversion URL.
Someone will need to convert every open ticket in review to a pull request.
I do anticipate some objections.
One objection is that each of the above tasks is going to take some work.
I am fairly confident that some of the people who have educated me here will step forward to volunteer to do it. Please reply to this message if you'd like to volunteer, saying what you'd like to volunteer to do. If not, then I guess that objection stands :-).
Another is that this might not be worth that investment of effort. This is why it was nice to have Alex contributing to the discussion: Django did basically this very change (right down to the "Trac for tickets / Github for pull requests" distinction), at a much higher scale than we have, and as he described it the change was *well* worth it.
Another objection is that Github is proprietary software, and an externally-maintained service that we'd be depending upon.
One solution to the "proprietary software" thing is the availability of the MIT-licensed <http://gitlab.org>. It's a largely feature-complete clone of Github; if, for some reason, we need to migrate away from Github in a hurry, it will be relatively painless to set up Gitlab instead, and the fact that Git is a DVCS means every contributor will serve as a backup. The main reason I would not suggest just deploying it is that it creates another sticky infrastructure-management problem, and while Braid is great, I'd prefer to avoid creating more work in that area. Github also has APIs for literally all of their features, so we can create a backup script.
(Also worth noting: Gitlab is an open-source competitor to Github, but they still trust Github enough to <https://github.com/gitlabhq/> host their own development there.)
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 <http://twistedmatrix.com/highscores/> a bit challenging, but I think it would be worth letting that break for the time being.
-glyph