A couple of other Red Gate developers, namely Mat Burton and David Conlin, and I were tasked by Jeff Foster, our head of software engineering, to try to improve the communication between the developers. We quickly came to the idea of code reviews — whether formal or informal, they tend to be fairly cheap and effective in spotting bugs or design mistakes. Perhaps more importantly, they let us learn from other developers, whether it’s by reading what somebody else has just written, or getting input on your own code.

We decided that our broad aim was for every developer in Red Gate to be able to do the level of code reviewing that they want. An aim of every developer doing code reviews felt a bit draconian — if a developer doesn’t want to do code reviews, but still produces great code, that’s fine by me. Our hypothesis is that most developers in Red Gate are already sold on the idea that code reviews are a Good Thing, but that they don’t happen for various other reasons. A quick poll sent round the developers confirmed that most developers either aren’t doing code reviews but would like to, or are already doing some but would like to do more.

With these results in our hands, some deeper investigation was required. This week, I’ve chatted to some of those developers who said they’d like to do more code reviews to find out what’s stopping them, and what might help them. One thing this taught me (or rather, reminded me) was that chatting to people in other teams is hugely enjoyable. From just a fifteen minute chat, I learnt about how that person works, how they’d like to work, and how they fit into the team.

The main recurring theme was that informal code reviews for each check-in weren’t viable. Although most people don’t mind being interrupted from their work to help solve a problem or answer a difficult question, they didn’t feel comfortable having to interrupt somebody every time they wanted to check something in. One team’s solution to this was to commit code to a work branch rather than straight to trunk, and to add an explicit code review step before “Done” in their process, meaning they’d have to get their code reviewed before it could be merged to trunk. This means that you can carry on committing code without requiring a code review, but ensures that all the code gets looked at by another pair of eyes before entering the product. The only snag they hit was that code waiting to be reviewed could pile up, leading to mammoth hour-long code reviews that nobody looked forward to. Their simple solution to this problem was to impose a WIP (work in progress) limit. In other words, once you hit a certain number of incomplete code reviews, you can’t start any new development work.

Another potential solution that Mat Burton has worked on is using Reviewboard to allow asynchronous code reviews. The idea is that you can upload the diff from your commit (or commits), and ask for a code review. Then, when somebody has a free moment, they can take a look at the code. One great idea that David Blurton had was to add some hooks to source control so that whenever your commit has a particular string, for instance “#codereview”, we’d automatically upload the diff for the commit to Reviewboard, meaning you don’t need to learn how to use another tool.

The potential problem is that the code reviews pile up. When code reviews aren’t mandatory, imposing a WIP limit isn’t going to work — people will then just avoid getting their code reviewed. Hopefully, making the number of pending code reviews easily visible, and making it as simple as possible to do the code review, should go a long way to avoiding this problem.

Hopefully, getting Reviewboard working reliably, possibly with David Blurton’s “#codereview” idea, will help some teams get the ball rolling. We’ll also be looking at other ways to solve some of the problems we’ve come across — suggestions are very welcome!