Back in the nineties, code reviews were painful, horrible experiences. You would be handed a thick parchment of source output and line numbers, and you had to go into a meeting for two hours to review all the code. The waterfall method at work.
Things got a little better when people would quickly send out diffs or attach them to a bug, but not by much. In the open source world, the reviews would go on for days with twenty levels of indents on long long email threads. At the end I'd scratch my head and wonder where things stood.
In my new job here at Symantec, I was introduced to ReviewBoard, an open source tool for code reviews. Wow. What a difference a tool makes. You make a change, and then use a tool to submit it for review. It gets the diff and submits it to Review Board. You then go in and specify who the reviewers are, submit, and you're done.
As a reviewer, you can look at the diffs in context with the source, and then click on any line and you can add a comment. When you're done, submit, and an email is sent out. The change owner can then go to specific comments and reply - each comment can have its own dialog which is logged in the tool. All this is done with AJAX style interaction, which is sweet.
The change owner can submit a new patch for review, and if they use the same changeset id, the tool lets you as a reviewer see the diff between the original and the updated version, so you can focus on what's changed since the last review. When you're satisfied, you click on "ship it" and you're done.
Atlassian also has a code review tool but (a) it's not free and (b) it doesn't handle iterative reviews - where you address comments by a few small changes. Their tool doesn't let you see the differences between iterations in the review, and that's pretty crucial when you have a large patch and the new revision only has a small set of updates. Also, it's difficult to submit a change before it is committed. So I think overall ReviewBoard is the better tool.
Because of this tool, we can now have a policy that every change be submitted for review before it's committed. Of course we are flexible with this, but this is the overall policy. I don't think such a policy would be bearable or enforceable without a tool like ReviewBoard. Thanks, guys!
No comments:
Post a Comment