Wednesday, October 8, 2014

Modern code review practices

I might just be curmudgeonly, but I'm about to the point where I won't even bother contributing to a project that doesn't use the Github pull-request model for code contributions.  There's so much pomp and circumstance with some projects, when they should be doing everything they can to welcome contributions.  Other similar systems like Bitbucket are fine, although Bitbucket's code review UI needs some help.  I've tried a lot of other systems over the years, and none of them match the simplicity and joy of Github.

The 90s called and want their code contribution practices back

I've recently begun trying to contribute to Ambari.  Ambari's contribution instructions read like a Microwave Oven's operator manual.  The process is so arcane that I'm surprised that anyone contributes.  You have to sign up for two services, JIRA and Review Board, and then you submit a patch to the JIRA ticket, as well as create a Review Board review with the same patch, and the same information as in the ticket, then link it back to the JIRA ticket and link the JIRA ticket to the review.  Here, read it yourself: Ambari's How To Contribute Page  I don't know if the Apache Foundation dictates this broken process or if the projects just take it on themselves to make the process horrible, but either way, someone needs to be informed that the 90s called and want their code contribution practices back.  Some of you might be saying, "what's the big deal?  Submitting patch files to two places and manually inviting people to review your code when you don't know who should review your code, then manually linking the review back to the ticket, and then enlisting yet another person to actually commit the code on your behalf isn't that bad" (if you're saying this, please punch yourself in the face and save me the trouble).  But there's also another catch that might be a little less obvious: attribution.  If you submit a patch, and then someone else actually commits the patch for you, the git log attributes the patch to the person who committed it, not you.  You may not realize this, but people like receiving credit for their work, even in Open Source.  Some people will simply refuse to contribute if they don't get credit, and frankly that's as it should be.  Can you imagine an actor or a writer doing work and then letting someone else put their name on it?  I can't.  Ambari puts your name on a contributors page, so it's not like there's no attribution, but that's not the same as having it show up on github, which is commonly used as a portfolio these days.

Barriers, barriers everywhere, and not a drop to drink

Openstack's contribution model isn't as terrible as Ambari.  It at least gets attribution right.  Openstack uses a system called Gerrit for code reviews.  You simply have to sign a CLA, sign up for Launchpad, download a magical tool called git-review (it handles all the arcane Gerrit shenanigans for you), and then run git-review on your committed local branch.  Then Gerrit takes it from there.  More info here: OpenStack's How To Contribute page You'll also need to read all of the coding standards, and all the information about how to format your commit messages, and oh yeah, make sure you know everything that's talked about on the mailing list because if you don't conform, your code will be rejected.  You'll likely not pass your code review and then have to re-submit the same branch for review (your new friend it git commit --amend).  Assuming you actually make it through the gauntlet of feedback and pass all the automated tests, which cover things like Openstack's extremely limiting and pedantic coding standards, then Gerrit will automatically merge it for you and push it up to Github.  So, it's usable, even if a bit overblown.  The biggest problem is that the process takes forever.  Even working with the same developer on an Openstack project and another project that's just on Github, it's amazing how much faster progress is made on the Github-based project.  I understand the need for verification of new code before it's released, but this is just a bit much.  Github lets you integrate with other systems to verify pull requests, and it works extremely well and doesn't require a ton of hoop-jumping from the developers.

I'll quit while I'm ahead

I really think it's antithetical to the spirit of Open Source to make it difficult to contribute to your project.  Github gets it right.  You fork my code, write your own code, then submit a pull request.  All the feedback and code is in the same interface, you can update your pull request based on feedback, and when it's good to go, the committer can merge it in.  Done.  If you don't want to use their issue tracker, you can integrate it with most popular issue trackers and have it automatically update tickets for you.  Whatever you do, don't make the contributor have to do this manually.

Full disclosure: I don't work for Github and I'm not a paid shill.  I just really think they nailed this process and there's a good reason they're becoming a de-facto standard.  I wish other projects would catch on.

No comments:

Post a Comment