Reviewing code can suck
The open source world has given developers great tools to make their lives easier. We have editors, bug trackers, source code management tools, repository viewers. Bugzilla, for example, is a very popular bug tracker used by many open source projects and companies alike.
While the life of a developer has in many ways been improved by these tools, there’s one key area of software development that people still do the hard way: Code reviews.
You’re probably familiar with this. A contributor puts a patch up on Bugzilla, Trac, or your project’s listserv. It’s large and spans several files. Eventually you get around to it (if you haven’t lost it in your inbox yet). You open an e-mail or the Bugzilla page to respond, put the diff where you can see it, and start going through the changes, line by line, making comments as you go. It’s tedious. You have to make it clear what function you’re talking about, make references to the general area in the diff. It’s a pain for the contributor too, because they have to figure out what you’re referring to. This leaves room for error.
We’ve been working this way for years in open source projects and at VMware. Code reviews are important to us, and help to keep our code clean and our products stable. It keeps us honest prevents us from cutting corners unnecessarily. However, the process is a bit time-consuming and not at all optimal.
- Generate an htmldiff showing the old code and the new code side-by-side, highlighting the changes.
- Write an e-mail to the target reviewers/listservs explaining what changes we made, the testing that was done, links to screenshots we put up on a file server, a link to the diff, and whatever else.
- Wait for someone to go through the htmldiff and comment on the changes in a new e-mail.
- Go through the comments, try to find the lines they referred to in the diff, and fix them up.
- Make another diff if necessary, repeat.
David Trowbridge and I finally got tired of it. We spent too long preparing review requests and we lost too many of them in our e-mail. As our personal projects and our team grew in size, it became harder to keep track of all the open review request e-mails on the listservs. So we fixed that.
Review Board
We built a code review system called Review Board. Like most projects, it started out simple, but grew to be pretty powerful and useful quickly. It was designed to automate and simplify the process of creating review requests and actually reviewing code.
Management at VMware was excited about this from the beginning. We weren’t being asked to write it, and it was a personal project and all, but when they found out we were doing this, they fully supported us. It didn’t take long for the excitement to spread across many teams, and we had 40 people signed up and playing before we were even ready to announce our phase 1 beta.
One of the things we decided from the beginning is that this must be open source and it must work with other version control systems. For example, we use Perforce internally, but there’s no reason not to include Subversion or anything else.
So we worked and worked in our spare time. After weeks of trial runs and lots of dogfooding, we decided it was time to announce what we had so far.
Enough yapping, details already!
Details? Alright.
Review Board is an open source program licensed under the MIT license. It was designed using Python and Django. It’s compatible with Subversion and Perforce, and can be extended to support other version control systems.
Review Board has a lot of useful features…
Multiple Repositories and Projects
Review Board can generate and display diffs against multiple repositories on multiple servers, each with their own version control system. At VMware, we have a repository for each Perforce server we use, and will soon be adding a Subversion repository for our libview project.
This would be particularly useful for umbrella projects where some parts are available in one repository and others in other repositories.
Writing version control system backends is easy. They’re simple Python modules that can be referenced in the Review Board database entry for the particular backend. We currently ship Subversion and Perforce backends, as well as a subclass of the Perforce backend making use of special extensions we use at VMware.
Review Requests
Posting review requests is fairly easy. If you use Perforce, it’s especially easy. A post-review tool is provided that allows you to post a review request with nothing other than a change number as a parameter.
If you use Subversion, a little more work is required, though we’re working on a tool to automate this as well. You’ll need to generate a unified diff. Click “New Review Request,” select the repository, enter the base path (the path relative to the root of your Subversion repository where you generated the diff), and then select the diff. Click “Create Review Request.”
You’ll be taken to your review request page where you can fill in description and other information. Click “Publish” when you’re done.
Powerful Diff Viewer
- Inline commenting: Instead of jumping to your bug tracker or e-mail client and describing where you’re commenting on, just comment directly on the line! Click the line number or click and drag to select multiple line numbers and a comment dialog will pop up allowing you to type. When you finally publish your review, the lines in the diff you commented on will be shown inline with the rest of your review.
- Inter-line diffs: Sometimes a small change is made to a line and it’s not readily apparent what the change was. In these cases, we highlight the regions between two lines that have changed.
- Revisioned Diffs: Each revision of the diff is saved and can be accessed. In the future, we will make it possible to show differences between two revisions, to ease review of incremental patches.
- Whitespace highlighting: Trailing whitespace is automatically highlighted. Trailing whitespace is a pet peeve of many developers and this helps to catch it early.
- Collapsing/expanding of files: By default, only the changed chunks and some lines of context around them are displayed. This can be quickly changed to show the entire file, in case there’s something you want to look at or comment on elsewhere.
- Keyboard shortcuts: Convenient shortcut keys allow quick navigation around the diff. For example, pressing “n” will jump to the next changed chunk, and pressing “p” will jump to the previous chunk. This will be further improved and documented soon.
Interactive Screenshots
- Screenshots can be easily added to the review request page, and will show up as thumbnails.
- Screenshots can be commented on by clicking and dragging an area on the image. A comment box will pop up, like in the diff viewer. While the clipped part of the screenshot does not yet appear on the review, it will soon.
Contextual Discussions
- Commented lines in the diff are displayed with their comments on the review page.
- Commented areas of a screenshot will be displayed with their comments on the review page.
- Discussion of reviews take place on the reviews themselves, making it possible to read the whole discussion of a change from top to bottom.
E-mail Support
All discussions and updates get sent automatically to the individual reviewers and listservs. This makes it easy for people to see any and all changes. This is configurable and can be disabled if it doesn’t fit in with your setup.
NIS Support
Review Board can use an NIS server as an authentication backend, making it easy to integrate into companies that use NIS. Everyone’s accounts will Just Work in Review Board without any registration required.
User Dashboard
All incoming and outgoing reviews can be seen on the Dashboard page. You can look at incoming reviews sent directly to you, sent to a group, or to either (all incoming reviews).
In the future, this will show more information on what’s going on with the review requests in your list. Recent discussions and updates will be shown, making it easier to keep track of things.
JSON API
Review Board has a JSON-based API for accessing nearly every aspect of the system. This makes it easy to build tools around Review Board. Our post-review tool uses this, as does much of the web UI.
Hopefully in time, developers will make use of this to better integrate with existing systems such as Eclipse, vim, emacs, or other IDEs.
Looking Forward
There’s a lot we have planned. Review Board is still just an early beta and a lot will happen.
- Improved tools: We’ll soon be working on a single tool that understands how to post review requests to both Perforce and Subversion. It will also be able to integrate with multiple projects. For example, if three projects you’re contributing to all use Review Board, post-review will ask once for the Review Board servers and use them from then on when run in the project directories.
- Interdiffs: We plan to include the ability to show changes between revisions of diffs, aiding in reviewing incremental changes to patches.
- Statistics: Information will be available showing how many review requests, reviews, and comments have been made between specified time periods, complete with fancy graphs. We’re working on ideas for what can be displayed here.
- Status Reports: This is perhaps more useful inside companies. At VMware, we’re supposed to submit weekly status reports discussing what we’ve done. Often this consists mostly of review requests made and bugs fixed. Review Board will be offering a page showing this information in several forms so that it can be easily copied and pasted into a weekly status report.
- Integrated Help: To simplify usage for first-time users, we’d like to add help information to all pages explaining simply how to use the interface.
- “Effective Lines” Display Many diffs are easier to review than they seem, as there could be hundreds of lines with nothing more than a function or variable name change. We would like to make this clear before even opening the diff by showing the number of effective lines changed, factoring out the simple redundant changes.
Wrapping Up, and Contributing
This was a bit long, but I hope it gave a good overview of what we’re putting together here. You can see it in action or visit the project page. If you decide to use it in your project, please let us know! Check the Wiki for install instructions.
Hopefully this will become useful to others as well. We’ll post periodic updates as this project progresses.
