Tuesday, November 3, 2009

Using software to do code reviews (ReviewBoard)

It's fairly universally accepted that all code should undergo some type of review. The problem is, while it's easy in theory, in practice it tends to be kinda tough. I've had various experiences participating in code reviews ranging from realtively decent to horribly painful and I still don't know that I have a perfect answer as to "right way to do things" (tm).

If I grab my "Software Engineering" book back from my college days, I can read about doing print outs of the code, having an official reader, while others follow along to make comments, and so on. No one I know has ever done anything quite so formal.

More often, it's grabbing a bunch of developers, and having the guy who wrote the code put it up on the projector, while people make various comments about his variable naming, or why he wrote

if(!boolOne && !boolTwo)

rather than

if(!(boolOne boolTwo))

or things equally silly.

In one particularly painful code review, I remember an "architect" giving a discourse on why magic numbers were bad. Fine, fair point. But after the first fifteen minutes, the point was made...yet we kept going for another 20 minutes. Yay!

The other problem, of course, is that while one should leave his ego at the door, it's kinda tough. While I think there's a lot of value in having devs *know* someone else will look at their code (The "Crap, if do this hacky thing, someone will see" effect), sometimes code reviews can be incredibly demoralizing if it just turns into a 'beat up on the developer' exercise.

Even with these caveats, though, I think code reviews are absolutely essential. But as challenging as code reviews are even with a bunch of motivated, full-time employees, all located onsite, it becomes even more challenging when your resources are located remotely.

What do you do then? Have everyone call in from India at 11:20pm their time using a web-ex and a conference bridge that's intermittantly goes in and out, while they walk through the code?

I've tried this and it's never worked out very well. It still had value, but man, it was *not* fun.

Given the fact that 80% of the devs writing software for my team are remotely located, I've struggled a lot with this question over the past few months. In fact, I've let things kind of just go on as they have, which entails the onsite dev leads waiting for CVS check-in emails, or notes from the offshore guys, then passing back comments via emial.

The problem with this, of course, is that it's ad hoc. Even putting some type of process around it (a checklist?), maybe an item on a Wiki page for the project to be struck-through, it still doesn't say anything qualitatively about the review. And there's no real audit trail. It's all via emial or the phone.

So, recently I started to look around for software that might be able to help facilitate this process. I've known about tools like SmartBear CodeReview for some time, but it's always tough to get approval for software that costs money. Mind you, I'm not saying it's impossible, but if you want to put something in place quickly at my company, it's not the way to go. Add to that the fact, that the company is looking at putting a company-wide installation of Microsoft TFS (Team Foundation Server) which includes some code review facilities and I have little hope of getting approval.

So, instead, I started looking at open source projects. There are a few options, but I ended up installing ReviewBoard.

ReviewBoard was created internally by the guys at VmWare so it's definitely more aimed at the Linux crowd. But with a little effort, I was able to (mostly successfully) get it up on an running on a Windows box. It's not without it's warts, but I think it might be quite useful in my current situation.

I can post all sorts of screenshots and so on, but instead I'll just post up what I did to get it running (in case you want to try it out). Hopefully this will be useful to someone out there (Keep in mind I'm not a systems admin, so please forgive any retardation in what follows):


  1. Download Xampp
    1. Xampp is an easy-to-install Apache distribution with MySQL, PHP, and Perl already included
    2. To install, simply unzip the file to your directory (I use C:\opt) and your good to go
  2. Download Python v2.5.4
    1. Subsequent releases are not currently supported in mod_python unless you want to build your own binary (And I didn't)
    2. Add a new system variable name PYTHON_HOME and set it to your install dir location, which should be something like: C:\python25
      1. Add the following to your PATH:
        1. %PYTHON_HOME%;%PYTHON_HOME%\Scripts
  3. Download mod_python-3.3.1
    1. Make sure the version you download corresponds to your version of Python (2.5.4 if you followed the instructions above), to your processor type (most people it'll be the 32-bit binary), adn to the version of Apache you'll be using.
      1. The version I used (32-bit, Python 2.5, Apache2.2) is here
    2. Edit the Apache conf (which should be located somewhere like C:/xampp/apache/conf/httpd.conf, find where the other LoadModule lines are and add this line

    LoadModule python_module modules/mod_python.so


    1. Create a simple test directory within XAMPP htdocs directory (which is where any deployed apps go by default) to test your install of mod_python. If you have any difficult with the steps as detailed below, see the instructions on the mod_python site here:
      1. In my case I created a directoy C:\xampp\htdocs\test
      2. Update your XAMPP install so it can recognize Python code, by updating httpd.conf. You'll want somethlng like this (note this is specific to the test directoy I created in the previous step)

        <Directory "C:\xampp\htdocs\test">
        AddHandler mod_python .py
        PythonHandler mptest
        PythonDebug On
        </Directory>

      3. Add a file containing the following to the newly create 'test' dir (Keep in mind this is Python, so spacing matters and you probably can't just copy and paste what I have below here)
        from mod_python import apache

        def handler(req):
        req.content_type = 'text/plain'
        req.write("Hello World!")

        return apache.OK
  4. Download the tarball of Django v1.1.1
    1. Extract the tarball to your directory of choice
      1. In my case this was C:\opt

    2. Goto the directory to which you installed Django and run the following command:
      1. Run setup.py install

  5. Download GNU patch.exe
    1. Run install (by clicking on .exe)
    2. Add the bin directory to your PATH system variable (i.e. C:\Program Files\GnuWin32\bin)

  6. Download the Python imaging library (Make sure you get the version appropriate to your version of Python. In my case, this was Python 2.5)
  7. Download PyCrypto
  8. Install SetupTools for Python.

    1. This includes EasyInstall which will make installing additionally needed Python components for ReviewBoard easy

  9. Use EasyIntall (installed via step 8) to install memcache from the command prompt:
    1. easy_install python-memcache

  10. Install ReviewBoard using EasyInstall
    1. easy_install ReviewBoard

  11. Install MySQL connector using EasyInstall
    1. easy_install mysql-python
    2. While ReviewBoard comes with an embedded SQLLite install, you probably want to use MySQL.
    3. This step actually failed for me as it requires VisualStudio 2003 (I have VS 2005, which doesn't work for this), so I'm using SQLLite for the moment

  12. Create your 'site' for your reviewboard installation, naming it as you'd like the site to be accessed. I used reviews.com and updated my localhost file so this would work (i.e. I access ReviewBoard on *my* computer via reviews.com)
    1. rb-site install C:\xampp\htdocs\reviews.com
    2. Source the ReviewBoard specific conf file in your global httpd.conf file
      1. The ReviewBoard conf file should be be here: C:\xampp\htdocs\reviews.com\conf\
      2. The Apache conf will be here C:\xampp\apache\conf, if you used the default install location)
      3. Here's the line you'll want to add:
        1. Include <install dir for ReviewBoard site>/apache-modpython.conf
  13. Create a .reviewboardrc file in C:\Documents and Settings\<username>\Local Settings\Application Data with the same name as your site (in my case "reviews.com")

    REVIEWBOARD_URL = http://reviews.com/

  14. Use TortoiseCVS to configure access to CVS via the command line (I'm going to leave this out unless someone complains about it, at which point I can provide more details)
    1. Download TortoiseCVS
    2. Generate SSH keys using PuttyGen
    3. Install your SSH public key on your CVS server
    4. Create the following file and name it "ssh.bat"
    5. @echo off

      call "C:\Program Files\TortoiseCVS\TortoisePlink.exe" -2 -i "C:\Documents and Settings\yourusername\sshidentity.key" %*

  15. Create a new Environemnt variable named CVS_RSH with a value of the full path to the "ssh.bat" file
  16. Add the CVS_RSH environment variable as well as the path to the TortoiseCVS install to your PATH variable

Pheww! Now, you're ready to start using ReviewBoard, which maybe I'll cover later.

5 comments:

Jeremy Walker said...

That installation process was nuts. Congrats on getting it running.

I find peer review incredibly valuable. I find large gathering of developers dissecting code to be destructive to team morale. It takes a really great team with a lot of trust to be able to create a useful feedback loop from those interactions. I have a fear that the common traits that make a programmer a programmer leads to less then effective interactions with the big group code review.

There are exceptions to every rule but I feel the only effective code review is one you have a "lead" developer open up their code to review with an agenda. Otherwise its too hard to control the crowd of alphas circling for the kill.

Code Monkey said...

Haha. Yeah, it took a bit of work, though mostly just the quantity of components required, rather than anything particularly difficult about any one component.

I honestly wasn't sure if I'd be able to get it running within XAMPP on Windows, so felt pretty good about getting it all running (more or less).

As for the your other comments, totally agree. Code reviews unfortunately turn into pissing contests as often as not. The developers involved are more concerned with showing how they are *smarter* than the person who originally wrote the code, rather than viewing it as a collective way to make the code better.

And since you can only hold people's attention for an hour or two, you tend to look at things from 30,000 feet and miss all the little edge cases and details...and those are precisely the things that were most likely to have problems.

I still do think team code reviews are a useful tool, but more and more I think offline reviews are more effective in most cases. Interestingly, that's pretty much what our "review checklist" back at our old company was--and it worked pretty well IMO.

Unknown said...

FWIW, on our web site we have tips on doing peer code review that are independent of our tool. In particular, a paper on dealing with the social effects: http://smartbear.com/docs/CodeReviewSocialEffects.pdf

Additional white papers/books/presentations here: http://smartbear.com/codecollab-white-paper.php

Jeremy Walker said...

@gsporar

Great paper. Ideally the leadership roadmap you've spelled out would work. In fact I've seen subsets of it in action work with some teams. I'll definitely pass that link around.

Code Monkey and I have run into the situation where programmers can't be guided into appropriate behavior or kicked out of the room. While what you've suggested can work, it requires execution by the manager and some cooperation from the inmates.

I'd have to try out the software to see if it helps with offshore reviews. Right now in an army of 2 I simply don't have the need.

Code Monkey said...

Thanks gsporar.

Funnily enough I have a copy of 'Best Practices for Code Review' from the SmartBear site sitting on my desktop. It and the PDF you linked too both have some great tips, but as Jeremy mentioned (as is the case with many things) some of those practices are easy in concept and far more difficult in the real world.

We tried to gently 'persuade' the guy who gave the 30 minute lecture on magic numbers and others in our group who were demeaning into better behavior, but some people just don't change. That's part of why I like the online tools.

Of course, as I said above, I still think in person reviews have a place too.