screeley.com

I went through the process of installing Review Board last night and was happily surprised and deeply angered at the same time. I'm pleased to report that most my anger was taken out on CentOS. Review Board is a Django powered tool for code reviews developed by VMware and subsequently Open Sourced. If you ever have some free time, browse around their repository, it's pretty impressive. I'll talk about my experience of installing it, but mostly I'll stick to the processes behind the code reviews. First off:

• Recommendations
  • Read the host requirements carefully before installing and note that you need to install pygments for syntax highlighting to work.
  • Set up your DB correctly before installing Review Board. You get three quarters of the way through the process before it prompts you to enter the DB information. When you don't have it, and you won't, you're going to have to start all over again.

Once you get through the setup, Review Board creates a few configuration files for you to use. If you don't install Review Board at the url root, you are going to need to modify the Apache configuration it creates. The media directories were off as was the location, but minor stuff. Kick Apache once and you have Review Board. I'd post the link, but the last thing I need is a posting titled 'The most f'd up Review Board'. I kid.

I guess what I didn't know going in was the process that reviews follow. It goes like this, taken from User Basics

1. You make some awesome change to your local checkout.
2. You create a review request by posting a diff, writing a description, and selecting some reviewers.
3. You click "Publish" on the review request and wait for your reviewers to see it.
4. Other people look at your review request, say "that is awesome, except some stuff is broken."
5. You update your code to address some of their comments.
6. You post an updated diff, and respond to their comments indicating what you changed (or you respond indicating why you're not going to make some change they suggested)
7. People look at your updated code, and give you the go ahead.
8. You commit your change to the repository.
9. You click "Set Submitted" on the review request to remove it from peoples' dashboards.

Review Board creates a nice little diff that allows you to comment on changes in line. This is a novel idea, people actually get their code reviewed before it gets checked in and they receive meaningful feedback on changes. Wow. Coding standards are awesome.

I work for a company that doesn't do a whole lot of code reviews. Once in awhile we sit around in a room and look at code that is already checked in. This very much defeats the process of a code review. From what I understand, it should be done before the code is ever committed, but in the world of deadlines and just in time coding that doesn't seem to work. That and no one wants to get in a room to talk about code. It's much easier to spot issues and raise questions when you can actually see the diff, not squinting at the 10 point font being projected on a wall.

I like that Review Board enforces a good code review process and have started to use it for more mature projects, but I fear the less mature projects will fall into the same routine.

If you haven't taken a look at it you should, even if you decide not to use it, it will make you think about your code review process.