Daily Code Reviews

August, 2009

We all know that code reviews are important right? But how often do you actually have someone review your code? How often are you responsible for reviewing code? How often do you actually take the time to read code that someone else has written and understand what it’s doing? I’ll be the first to admit that it’s been a while since someone reviewed my code. I usually seek out peer review only when things aren’t working, or if I want to show off some cool new language feature or pattern that I just learned. Otherwise, code reviews are hard to come by. They require that you disrupt someone else from their job to come look at your code. And believe me, it’s scary having someone look at your code. I look at code I wrote 6 months ago and think ‘What was this idiot doing? - oh wait, that’s me!’ I think that code we write can become very personal, yet one of the best ways to improve as a developer is to have to show that code to someone else and explain what you were trying to accomplish and why you made the decisions you did. Sometimes the hardest problems can melt away just by explaining that problem out loud to someone else. I can’t tell you how many times I’ve called a co-worker over to my desk to rant about some issue or bug that I’m trying to solve and before they even get a chance to speak or fully understand the issue I’ll have come up with the answer. Just the act of saying it aloud and explaining the problem to someone else is enough.

We are rolling out a new line of mobile products here at Blue Dot and I was pulled into one of these projects to review code and help get stuff out the door at the last minute. The more I thought about it over the week, I wished that we’d been doing detailed reviews over the course of the development instead of waiting until the end of the project. But how do you enable this? How often should we review code? Once a project? Once a sprint? Once a day? What’s keeping people from seeking out peer review? Who should be doing the reviewing? Who should get reviewed?

In response to all these questions, this is what we came up with: Our dev team is going to strive for daily, cross project, peer code reviews. Let me explain some of the key factors to this. First, I’d like to see reviews happen on a daily basis. Every day I’m responsible for reviewing someone elses code and someone is responsible for reviewing my code. I don’t think these need to be massive code reviews and if they are truly happening on a daily basis there is only so much code you can write between them. Some days there maybe little or nothing to review. The key to reviews happening daily is that I want to catch things quickly and get developers comfortable with the idea of reviewing and being reviewed. This should create a small feedback loop so that if on Monday one of the action items out of your code review is to not hide exceptions, by Tuesday’s review (which you know is coming) - you’re going to be very aware of not making that mistake. Second, these reviews are done cross project. What this means is that the code you are reviewing is not necessarily from a project that you are actively working on. At Blue Dot we have a pretty small development staff, but we work on a LOT of projects at once - often with only 1 or 2 developers on a single project. The idea of cross project reviews is that you get to see what everyone else is working on and this will setup and environment of cross project pollination. If you are working on project A and need to implement WCF streaming, the cross project review system is going to enable the appropriate communication for you to discover that project B has already solved this problem and can just reuse and/or improve upon their solution. Even with our small development team it is amazing how often we solve the same problems over and over again because people just don’t know or have time to know the details of other projects. Finally, code reviews are to be done by peers and on a daily basis you get to act both as a reviewer and as a reviewee.  Even if you aren’t a senior developer being a code reviewer gives you the chance to see new code, ask questions, raise concerns and maybe even point out something you learned earlier when your code was reviewed. For more experienced developers this is a great chance to provide mentoring in a structure environment, talk through advanced issues and features, see what common problems are across our code base, and begin to understand where more training is needed or where better guidance should be provided. One last thing that I think is key to all this is having some sort of formal structure of being specifically assigned to review a particular persons code. We are going to start by rotating our assignments weekly - so for one week you will have the same person reviewing you code (and you will in turn review a different individual all week). The reason this is important is because it removes the barrier of collaborating with someone you potentially don’t know well or are afraid of disrupting. It’s also  nice to have a week’s worth of continuity to see improvements and check up on specific weak points.

I’m excited to see where this goes, and plan to post progress, results and the evolution of this process on this blog. I don’t have or want to have any specific rewards tied to developers participating in this as I think the motivation to write better code and build better applications is enough. If nothing else, I’ll bet the people that really take advantage of this are going to quickly rise to the top in terms of knowledge, code quality and success in their individual  projects. Stay tuned!

Tim's Avatar Building GitHub since 2011, programming language connoisseur, Marin resident, aspiring surfer, father of two, life partner to @ktkates—all words by me, Tim Clem.