How NOT to Run a Code Review

Throughout the rest of this article series (which begins with Running an Effective Code Review), I’ve presented code reviews as a normal part of the technology development process. I have intentionally sidestepped the many ways in which they can become meetings from hell: politically, technically and in terms of process.

THE ULTIMATE GUIDE TO CODE REVIEWS

Running an Effective Code Review

5 Reasons for Software Developers to Do Code Reviews (Even If You Think They’re a Waste of Time

What to Look for in a Code Review

Making Code Review Software Tools Help, Not Hinder

How to Lead a Code Review

How Not to Run a Code Review

Doing Spot-On Code Reviews with Remote Teams

By far, the most damaging of these is politics. New developers should be cautioned that, especially in large companies, 95 percent of code reviews are venues for political posturing, says longtime developer Chuck Brooks.

Developer Adwait Ullal (and several others) cite situations in which code reviews were used as a witch hunt, either in terms of personal mudslinging or performance reviews leading to termination. Christopher Phillips, director of software engineering at IAC Consumer Applications & Portals says, “I’ve seen code reviews done to pave the way for letting people go. I’ve seen reviews used to ‘put people in their place’ in the perceived pecking order.”

Sometimes the problem is in the developer’s head, not the reviewers’ attitude—but that’s all the more reason to ensure a code review leader adopt the role of mediator. Former technology chief Jay S. Hemmady’s worst code reviews were where insecure coders felt threatened during the code review. “Everything went horribly bad soon after the meeting started: emotions, sense of personal attacks, defensiveness, etc.” Old wounds from previous personality clashes can surface resurrect “us versus them” feelings. All in the name of improving the code, mind you.

Oliver Cole, president at OC Systems and also the lead for the open-source Eclipse Test and Performance Tools Platform project points out that the worst code review experiences always involve ego. “Some people cannot take criticism. Some people cannot give criticism. Furthermore, some people take a code review as a time to make it clear to all how superior they are to others.” He adds, “These reasons alone are enough for previously peaceful countries to go to war. Now add in the fact that there are precious few objective measurements for what is ‘good’ in code and you can get major disagreements that cannot be objectively debated or decided.”

Smartbear Software’s president Jason Cohen sums it up: “The worst morally and morale-y is when reviewers use criticism as a weapon. Some people laugh (nervously) when I describe it like that, but it’s quite serious. However this is usually extreme enough that everyone knows it’s happening, so a manager who is paying attention can root out this problem pretty easily.”

But politics is not the only danger in running an effective code review. There are other places where well-intentioned teams can fall into the quicksand.

Failures Beyond Politics. Oy.

Madison Decker, senior technical lead at InfoCision Management, cites one instance in which a code reviewer never met with the developer whose code was reviewed. All correspondence was sent via e-mail, which can—and did—lead to misinterpretations over what was being said. “In addition, [the reviewer] only pointed out the problems with the code. He never gave recommendations to improve or pointed out good things,” says Decker. The upshot? The developer left the company.

Sometimes, the code review fails because participants get caught up in the details. EMC distinguished engineer Steve Todd says, “Reviewers should be aware that the main goals of a code review are correctness and maintainability of the software.” But while that’s important, “I have witnessed excessive focus on aesthetic issue in code reviews that has dragged the entire review process into a quagmire. For example, one software developer actually used a ruler to measure line length to see if exceeded 80 characters! The whole review process focused on issues such as this and missed the point on code correctness and maintenance.”

Similarly, code reviews can get lost in the process of gaining code statistics. Explains Micheal Lalande, director of technology at QLogitek, “These are usually done through automated tools, and will focus a developer on avoiding false positives in the name of getting better statistics. This is the worst thing that can be done, because there are some very valid reasons why you may break a rule.”

For Paddy Sreenivasan, the VP of engineering for Zmanda, the worst experience was “when a code reviewer began making code changes instead of making suggestions back to the original developer.”

Then we get back to basics: code reviews that forget their purpose, and then lead to politics. “Mission creep can easily come in,” says Cole, “Where the design (which is not really supposed to be part of the code review) comes under criticism. This can spin out of control real fast.”

E. William Horne, systems architect at William Warren Consulting says the worst sin of code review is that it becomes an exercise in avoiding change instead of making sure that changes are implemented well. “Some employees know just enough about computers to appear competent, but are deathly afraid of having to actually study and understand any method, procedure or result that’s outside of the experience set which got them to the point where they were invited to review someone else’s code in the first place.”

Genesis

In the beginning, there was the system. And the system was null and void.

And the guru moved upon the system and said, “Let there be code.” And behold there was code.

And the guru said, “Oh crap, it doesn’t work.” And the guru called upon the code angels and demanded a code review.

For more uplifting advice, see the other articles in this series:

• Running an Effective Code Review (this article)

• 5 Reasons for Software Developers to Do Code Reviews (Even If You Think They’re a Waste of Time)

• What to Look for in a Code Review

• Making Code Review Software Tools Help, Not Hinder

• How to Lead a Code Review (let’s set up and lead the review)

• How Not to Run a Code Review

• Doing Spot-On Code Reviews with Remote Teams