Formal Code Review Process is No Good

Perfunctory is common. There are better alternatives.

With Git’s Pull Request feature, many so-called ‘Agile’ teams introduced a formal code review process. While a “PR review” process might sound good (in theory), it often turned out to have little value for too much effort. In my opinion, not worth it. There are other much simpler and more effective ways, if the main goal is to prevent bad code (mostly in a form of introducing defects and bad design).

Two years ago, I already expressed my opinion after observing many “LGTM” (look good to me, i.e. don’t care) style code reviews. Fake Code Review with ‘LGTM’.

Think about it, the developers even abbreviated the request for approval (Code Review), will they examine the code carefully? Of course not. My view is not alone, here is a funny picture posted on LinkedIn, with 5951 likes/reactions.

Source: https://www.linkedin.com/feed/update/urn:li:activity:7039897678494740480/

Frankly, a “fast-food” style code-review is a joke and an insult to software engineering.

Typical Problems with a Formal Code Review Process

Code Review, as a concept or in a light/casual fashion, is not bad, but a formal code review process is almost certainly wrong. Here are typical problems:

• Introduce interruptions
  as we all know, most programmers don’t like interruptions. Reviewing PRs means a lot (& ongoing) interruptions.

• You (either the committer or the reviewers of a bad change) feel guilty.
  The manager might ask you two to explain. But you both (and the whole team) know that this will happen again. The process is wrong!

• You want to move on to the next task, but have to wait for others to approve the commit (or PR) after the ‘code review’.
  Time-wasting.

• You receive wrong feedback (such as a particular coding style) but are frustrated about whether to accept or debate.
  Lowering the team’s morale and fostering the “Don’t care’ attitude.

• When seeing a ‘code smell’ (bad code, the term is from the classic ‘Code Complete’ book) by someone, you are hesitant to tell him because he is not ready to receive feedback.
  This is against Agile’s Collective Code Ownership’ principle.
  Also, by doing that, you will upset more than one person, the reviewer as well.

A common sense question, Can a software engineer A (typically wearing a noise-cancelling headphone) stop his/her work, eyeballing on the code changes (line by line) in a Git PR by B, then find out all of bad code? A reminder: this is not happen-once activity. “Formal Git PR review” is naive for two simple reasons:

• Technically: Oversimplified coding. Code is interconnected. A PR only contains a small section, a quick looking at them is surely not enough.

• Humanly: the software engineer A really wants to continue working on his/her stuff (and puts back on the headphone).

Why is a formal code review process popular (again)?

The formal code review process is not new, and it has been proven ineffective. That’s why Agile introduced the ‘Pair Programming’ practice (in Extreme Programming), which I will cover shortly.

The reason code review’s regaining popularity, in my opinion, is due to Git, in particular, its associated Pull Request feature (marked by Github). Ironically, Linus Torvalds, the creator of Git, called it “pure garbage”.

So, PR, at least from Linus Trowalds’ view (which matters a lot), shall not be the technical reason to enable effective “Code Review”.

A ‘Popular’ process does not mean ‘Right’

Some will argue, “Most software companies do formal code reviews, how could that be wrong?”. Popular (for a time period) ≠ Right.

A classic example is the infamous Rational Unified Process (RUP). By the way, one of the main motivations for Agile is to clean up the RUP’s mess.

IT managers or whiteboard architects tend to over-simplify software development with obviously silly (but widely adopted) practices, such as:

• Hiring more programmers, in the middle or later stage, to finish development work faster.
  
  However, this practice is widely known to be flawed and has been discussed in the classic book “Mythical Man-Month” almost 50 years ago. Despite this, it’s now still frequently observed in practice.

• Using a fancy project management tool in the hope of greatly making software development instantly better
  
  I’ve observed team members spending more than 50% of their time using Jira/Confluence, which is someone else’s software. Insane! Furthermore, new Agile Project Management tools such as Monday and ClickUp claim that “Jira is bad,” but it turns out that these app developments were quite bad too.

“These companies (Asana, Monday, Smartsheet, and ClickUp) can’t even make their own businesses work, yet they’re trying to sell you software to run yours.” — DHH’s an insightful comment on LinkedIn (2022–11–10)

• Estimating User Story Points, and use that as the base to manage development ‘velocity”.
  
  Ron Jeffries, the creator of ‘User Story Points’ and a co-author of Agile-Manifesto, has already ‘apologized’ for that. For more (quotes from the experts), check out my article, Estimating User Story Points is a Waste of Time.

• Excessive meetings under fancy banners, such as daily standup, sprint-planning, user-story estimation, task decomposition, retrospectives, …, etc.
  
  Ironically, the most useful one, Sprint ShowCase, is often dropped after a few sprints. The reason: the team is unable (or lacks confidence) to demonstrate. (Check out my solution: Automation-Assisted Showcase).
  
  While many IT executives acknowledge “meeting, in general, is bad and time-wasting”, it is so common that there are so many meetings every week. Check out Jason Fried (in his famous TED talk: “Why work doesn’t happen at work”): management and meetings.

There are many more examples, I will stop here, but you get the message.

The Purpose of Code Review

Some will argue, “Yes, I agree with the problems with the formal code review process, it is expensive, but it did pick up some coding issues every now and then.”

Let’s go back to the root with a simple question, “Why do you (a software team) do code review?”

A typical answer: “Find the coding errors or bad code earlier, because the earlier detection means easier, cheaper to fix”.

Yes, the answer (the goal) is correct. My next question, “Is (formal) Code Review is the only way to achieve the goal?”. From my experience, there are better alternatives.

Better Alternative to Code Review Process: Pair Programming