You’re probably doing code reviews incorrectly

The article Processes and rules make code review less intimidating by Stefan Judis writes,

“Code reviews are, by nature, intimidating. Sometimes even brutal. If you’ve been in the game for long enough, you probably experienced the following: you worked hard on a feature, you’re proud of yourself and open the PR to be praised and land your changes, and then… it rains comments, suggestions and nitpicks. And if it’s really bad, you’re forced to take multiple feedback and clean-up rounds. It sucks.”

Oh, wow.

Why are people treating writing code like individual school assignments?

That is absolutely not how to effectively use code reviews. That is absolutely not how to work in a team. Teams work together, not against each other.

Why are you putting your heart and soul into your solution? Why is it “your” solution? Is this how y’all were raised? You know, where you prefer being the star genius in your own story, where you managed to get the perfect solution on the first try? Like, you’re the superhero, brilliant, engineer, billionaire playboy?

And then, you learn that you aren’t, and you’re shattered.

But you know what? You’re on a team that’s willing to look at what you made and really try to make it better. Maybe they will make it better! Or maybe they won’t. Both are good! If they do … then it’s better. That’s a win. If they don’t, well, then, you’ve gotten some evidence that supports your theory that what you’ve written really is good and will work.

Up until someone looks at it, you only had a hypothesis that your solution was good enough. You suspected it because you had some code and you had some tests (yes you did, otherwise you have no right to be offended about code-review comments). That’s a hypothesis.

You know who else does reviews to verify theories and hypotheses? Scientists.

Quit your whining. Quit your bullshit.

If you’re treating code reviews like a gladiator arena, as if you were going on Shark Tank or The Voice, then you’re doing it wrong.

The best software is written by a team. It is collaborative. Maybe one person is writing all of the actual text, but there are other minds that contribute advice and feedback that hones the final product.

You know what that sounds like? A writer and one or more editors or proofreaders.

This is how professionals work. Fix your process. Fix your expectations. Fix your fragile ego. Seek validation in a less self-destructive manner.

The author proposes a fix. It’s a technical solution, so it’s not great.

He could have suggested that people do “live reviews” instead of PRs because most people are too lazy or incapable to write critical comments that are also constructive. This is sadly often the case because learning how to write and how to empathize is a lot of work. You could start with empathizing, about which more below.

Instead, he writes about an insipid system where shittily aggressive review comments like “this is not worded correctly” are somehow made better by prepending the text “suggestion:”.

No. It does not make it better.

Why not? Well, for starters, because the text is not formulated as a suggestion. There is only an implicit suggestion that the reviewer would have worded it correctly. This is passive-aggressive time-wasting behavior. On top of that, everything in a code review is a suggestion unless the power dynamic in your team is so severely skewed that we need to be having a different conversation.

The comment should read something like, “I think that something like “… …” would be a clearer way of writing that.”

Or, maybe, you could establish a rapport with the people reviewing your code so you’re not pants-shittingly terrified that you’re going to lose mana when you do one. Maybe you could—gasp!—even be friends. This would help establish a human connection often summarized as empathy wherein the reviewer would consider for a brief moment how a comment might look to the other side and adjust it accordingly, in a manner that is totally not like how robots would do it.

If you’ve established that code reviews are collaboration and not a gladiator arena where “two enter and one leaves”, then the reviewer can be more concise without wasting a lot of time writing curlicue sentences. If you don’t have this rapport, then, yes, I’m afraid you’re going to have to be…what’s the word?…polite.

If you can’t be polite, then, at the very least, you should write review comments that don’t need review comments of their own. You’re going to have to follow the rules of error messages. As detailed in Alerts (Apple HIG),

“Avoid using an alert merely to provide information. People don’t appreciate an interruption from an alert that’s informative, but not actionable.”

Any review comment has to be both informative and actionable. The comment in question—“this is not worded correctly”—is neither. It just vaguely points at something and says “you suck.” It’s clearly attached to a specific line but doesn’t indicate what’s wrong with it. Even if it said specifically what’s wrong, it doesn’t suggest how to fix it.

An error message from a piece of unthinking software can’t go quite that far—unless it’s a spellchecker or grammar-checker!—but an actual, empathetic human in the role of a collaborator can! That person could formulate a suggestion for how to rewrite it so that the review for that line might end after only one cycle. And, as a bonus, it doesn’t end with anyone crying and curled into the fetal position under their desk.

Even if you have an informative and actionable comment, we still come to another downside: they’re still not very efficient. The most efficient way of reviewing code is to do it synchronously or “live”, where the collaborators can discuss and improve the code on the fly, together maybe—and here me out here—even before you’ve even pushed anything! Imagine!

If you’re stuck using PRs and web UIs to communicate, then writing comments like the one in question just wastes everyone’s time. The submitter either will assume what the commentator meant and try again—NOPE, STILL WRONG—or they will have to write “what do you mean?” or “how would you have written it?” This is useless churn. Just write your reformulation with your comment. Remember, you’re a collaborator. You’re not just trying to get through this review as quickly as possible. It’s part of your job.