Rietveld update, or “How to make code reviews fun (without canceling them)”
I have a new mistress and her name is Rietveld. I call her Veldy or sometimes, in the throes of passion, RV. But only because I have no idea how to pronounce Rietveld out loud.
In a recent post, I talked about our switch to the code review tool. And by recent, I mean less than a month so it’s been a torrid affair but I can tell she’s the code review tool I’ve been waiting my whole career for.
In that post, I described our old process and the proposed new one. Now, a few weeks later, I can begin my mission to make sure every man, woman, and child knows about the tool whether they do code reviews or not.
Our process is the same as outlined in the previous post but I’ll mention some side notes here just to ensure there’s new content in an attempt to keep potential trolls at bay.
The absolute greatest feature by far of the tool is how it enables asynchronous and disconnected reviews. By “disconnected”, I mean I don’t need to have the code with me. (You still need an Internet connection.) I just finished a ten day trip to London* and performed at least half a dozen reviews during that time on my wife’s laptop. Everything I need is included in the review page: the old code, the new code, and any comments by other reviewers or the initiator.
There are a couple of things this has enabled. First, I can do code reviews any time I have a free moment. I whipped off at least four this morning. Before I hit the sack for the evening, I’ll often open Rietveld up and bang out one or two just to end on a high note. It’s rare that I spend more than half an hour on any of them. More often, it’s closer to fifteen or twenty minutes.
If you’re wondering why we have so many reviews, that’s the second “enabled thing”: It’s so easy to do them, we can now review everything. We even have our own process swimlane for stories that are to be code reviewed. I.e. a story can’t be marked done until it’s passed the review. Ditto for bugs. The reason this works? Because it’s trivial to start a review and it’s almost trivial to do them. At this given moment, we have fifteen active code reviews. I expect at least three of them to be closed in the next twelve hours. But I’m also expecting two more to be created in that same time.
And because it’s asynchronous, I don’t even need to finish a review when I start it. Any comments I’ve made that session are stored as drafts until I publish them. I’ve done this a couple of times: Hit some code that required more thought than I was able to give at the moment so I just shut down the review and moved on to the next one.
There is a nice side effect that’s come from this process. Our team consists of one junior developer, a senior developer, and me. Because the comments are persisted and because we don’t allow stories to continue until the reviews are complete, the reviews have become a good source of training and mentoring for the junior dev. She will submit a review for a story, launch a review, and then update the code several times addressing issues that were brought up during the review. Sometimes the comments are trivial, like “make sure the indentation is consistent”, sometimes it involves a fairly sizeable chunk of work, like “add unit tests to expose the bug”. During this time, she can submit further patch sets and Rietveld can show you the diffs between them and the previous patch set. All the while, we (and by “we”, I almost always mean “the senior dev”) are providing on-the-job mentoring that’s relevant and contextual.
“Code review” no longer holds a connotation of six people in a room for hours being kept conscious by whatever substance passes for coffee these days. Now, it’s become a relatively benign task, like verifying we have unit tests. Story’s done; launch a review and move on to the next one.
So if you meet the requirements (use SVN, Hg, or Git; can create an app on AppEngine), you’d make an old-ish Hillbilly mighty happy iffen you’d check it out.
Kyle the Renewed