Improving Code Reviews

A "code review" activity is part of many organizations' development process - and oftentimes, it sucks. It frustrates people, wastes their time and the value in improving quality is questionable at best. If you feel that's the case, here are some things you can try.

What's a Code Review?

"Code review" is a feedback process, fostering growth and learning. It should not be confused or conflated with a QA sign-off process. While finding problems in the code may be part of doing the review, that's not the key point.

So-called one-way "gate reviews" without feedback on defect-free code are a waste. A major portion of their value is missed. The best reviews won't merely help people learn where they messed up - they help people find new, better ways of doing things!

Now, let us explore five common antipatterns and what we could do about them.

Five Code Review Antipatterns and how to deal with them

Review Hierarchy

In many organizations, the Code Review process "puts people in their place": A more senior person reviews the code of more junior persons, and annotates everything that these did wrong. Yes - this sounds exactly like a teacher grading a student's term paper, and the psychological implications are very similar.

While this does indeed foster some form of learning, it creates an anhedonian mindset: the key objective of the coding developer is to avoid the pain of criticism and rework. There is little joy in a job well done. Deming's 12th point comes to mind.

Suggestion 1: Reverse the review process. Let the junior review the senior's code, and see what happens.

Suggestion 2: Do review round robins. Everyone gets to review everyone else's code.

Suggestion 3: Have an open conversation, "How do Code Reviews affect our view of each other's professionalism?"

Huge Chunk Reviews

I'll admit that I've been both on the giving and receiving end here: Committing huge chunks of code at once and sending thousands of lines of code for review in one big bulk, without any comments. And the review outcome was, "This is garbage, don't work like that!" Rightly so. Nobody in their right mind has time to ponder such a huge amount of changes in detail. The review feedback will take a long time and probably not consider all the important points - simply because there are too many.

Code Reviews shouldn't create a huge burden, and they should have a clear focus.

Suggestion 1: State the review objective: What would you like feedback on?

Suggestion 2: Send increments into Code Review that can be thoroghly reviewed in no more than 15 minutes.

Suggestion 3: Reduce feedback intervals. For example: no more than 2 days should pass between writing a line of code and getting it reviewed.

"LGTM" or whimsical feedback

Poor reviews start with the premise that "the only purpose of a review is to find problems." On the positive side of the spectrum, this leads to a lot of a standard "lgtm" (Looks Good To Me) annotations as code is simply waved forward. On the opposite side of the spectrum, some individuals feel an almost sadistic need to let others know that there are always problems, today stating "this is good", and tomorrow stating "this is bad."

Behind this antipattern is the "controller mindset" that someone in the organization believes that a review is intended to tell others, "you did this wrong, you did that wrong." 

You can improve this by moving away from checking the code towards positive reinforcement, creating virtuous learning cycles. 

Suggestion 1: Change the guiding question from, "What is wrong with this code?" towards, "What could I learn from this code?" 

Suggestion 2: Create Working Agreements how you want to deal with extreme ends of the review spectrum.

Suggestion 3: Collect the learnings from Code Reviews and look at them in the Retrospective.

Ping-pong or Ghosting

Small story: One of my teams had just fixed a production problem that was causing a revenue loss of roughly €15k per day. Someone from a different team did the code review, demanded some semantic fixes, these were made - next round: lather, rinse, repeat. After 2 weeks, the reviewer went on vacation without notice. The fix got stuck in the pipeline for 5 days without response. This funny little event cost the company over €250k - roughly three years' worth of developer salary!

Things like that happen because the expectations and priorities in the team aren't aligned with business objectives and also because of a phenomenon I call "ticket talk."

Suggestion 1: Make the Wait Time and Flowback caused by Code Reviews visible.

Suggestion 2: Create a Working Agreement to talk face-to-face as soon as there's flowback.

Suggestion 3: Replace Code Reviews with Pair Programming.

Preferences, emotions and opinions

Let's return to the "whimsical feedback" antipattern briefly. Many times, I see feedback over "use CamelCase instead of Snake Case", "use Tabs indentation instead of spaces" or whether a "brace should open behind the method name rather than in a new line". 

None of these make the product any better or worse. The debate over such matters can get quite heated, and potentially even escalate into a full-blown religious war. These are entirely up to personal preference, and as such, not worth a review annotation: They are red herrings. 

Suggestion 1: Formalize coding conventions and put them into your Lint / SCA config.

Suggestion 2: If you're really bothered, use a pre-commit hook to prevent checking in code that violates static code rules.

Suggestion 3: If you think a rule is missing or unproductive, bring it up in the Retrospective.

Alternative perspective

Code Reviews are just one way to improve coding within a team and/or organization. Mandatory code reviews - by default - create interrupts in the flow, reducing overall performance by a significant amount. Better options include:

• Code Review upon request
  (e.g., "I want to talk with you about this code")
• Code Dojos, where the entire team assembles to learn from one another.
  (SAFe's IP Iterations are great for dojos.)
• Pair programming - making the discussion part of the process.
  (Reviews should be obsolete if you do Pairing right)

Still, if your organization mandates code reviews, try to make the best from them.

Summary (tl;dr)

Code Review is more about fast feedback learning than about "catching errors".
A positive "what can I learn" attitude makes reviews much more enjoyable and beneficial than a negative "what did they do wrong" attitude.

When reviews expose pressing problems, don't just annotate them. Engage the discussion about "how can we work differently?"