When Peer Reviews Attack

I'm a big fan of peer review in software development. I love sharing ideas, getting feedback on my work, and finding ways to improve my code. But in over 7 years as a developer, I've never seen reviews be done as often as they should. And, sadly, several that I have seen and participated in have gone horribly wrong. The failure usually boils down to poor communication. A bad review meeting, or chain of emails, can degenerate into finger pointing, hurt feelings, and ultimately failure to achieve the goal of the review process: high quality code.

Why review?

Peer review brings many benefits to a software project. It's an opportunity to have another pair (or more) of eyes on a developer's work, trying to catch subtle (or simply careless) mistakes, suggest ways to improve code, or ensure adherence to standards. Peer review is collaborative; the reviewer is taking a stake in the original work and will be backing it with their own seal of approval at the end of the review process. Problems will be caught sooner, standards will be followed consistently, and both reviewer and reviewee will have a chance to learn from the collaboration and perform better in the future.

What could go wrong?

Plenty. For example:

• Expectations for the process aren't set up front. A developer who doesn't know what to expect from the process may not be prepared to participate properly.
• Standards aren't communicated in advance. Peer review shouldn't play out like a game of Mao.
• The reviewer is overzealous. Peer review isn't a time to pursue one's own programming ideaology. That's what the standards committee was for!
• The reviewee takes it personally. Remember that it isn't about you or your worth as a developer. It's about working together to produce high quality code.

I've seen each of these dramas. They lead to alienation of team members, reduced productivity, and sometimes the abandonment of the entire review process. But I've also seen code reviews that work. Good reviews allow ideas to flow freely and the result is ultimately a better solution. Even if I'm only sitting on the sidelines of a review, I can learn something from the process that makes me a better developer.

What makes a better review process?

• Document the process. Make sure developers are aware of the review process before they begin any work that will be reviewed. If possible, have a new developer sit in on a review first. Reviewers and reviewees should know how the process will work, what they're expected to bring to the process, and what the expected results are.
• Document the standards. Make sure you have clear, reasonable, and up-to-date standards documentation available to the reviewee before they begin any work that will be reviewed.
• Be specific. The reviewer should point out not only which rules were violated, but also where. Give the file name, line number, and a brief description of the violation.
• Be flexible. The reviewer should ask questions and suggest alternatives, but keep in mind that there's a difference between a violation or bug and an option he or she might prefer (if you can't prove empirically that it's better, then it isn't a blocker). The reviewee should do his or her best to follow standards and document code where necessary, be ready to fix problems that are found, and be open to considering suggestions.

It's a jury of your peers

I think this one point is worth stressing. In the best reviews I've seen done, everyone is treated as an equal. It isn't a senior developer lecturing a junior on proper technique, it's two (or more) developers who may not be of equal experience or skill, but nevertheless regard each other, and the process, with respect.