How To Be A Nerd Without Ruining Code Review

Friday. August 11, 2017

Code review trips up a lot of developers. It certainly gave me trouble–and it still confounds me on a regular basis. In my last post, I provided some of the background for this struggle. Developers are almost exclusively nerds, and nerds come from a system that encourages them to become self-righteous and unsympathetic. It teaches us to value logic and being right above cooperation and humility.

As systematic beings, we would prefer to wall ourselves off from humanity and communicate only with the computers. After all, they never question us or disagree with our commands. But whether we like it or not, we’re living in a world where human interaction is a vital skill–even in the process of building computer software. Hey, I agree with you! It’s a major bummer. But it’s one we might as well learn to deal with.

Misunderstood

Our non-developer coworkers and managers clearly misunderstand us. Otherwise they wouldn’t place such unrealistic social expectations on us. They wouldn’t throw us into this highly nuanced, highly collaborative lion’s den known as code review without at least giving us some coaching first. Without at least warning us first of the perils of miscommunication, of pride, of rage. But no, they just leave us on our own to exchange criticism with other humans. As if this were a basic task that should come naturally to everyone.

The Process, In Theory

To a highly logical thinker, the abstract concept of code review makes perfect sense. First, a developer must present a draft of changes (a diff, or pull request) to his colleagues for review. Subsequently, said colleagues inspect the code for imperfections. Upon discovering a potential imperfection, a reviewer places a comment describing the issue on the offending line. It is then up to the code author to address the reviewer’s concern. There are two ways a developer may address a comment. The first is to amend the pull request to incorporate the reviewer’s suggestion. The other option is to refute the comment with extra information or an explanation of the choice. Once all imperfections have been found and all comments have been addressed to all parties’ satisfaction, the reviewers agree to merge the pull request.

Code review is how modern software gets built, pull request upon pull request. It is the best team software development methodology, proven through decades of experimentation at countless organizations. It’s easy for any developer to see how it’s such an effective process.

The Process, Through A Developer’s Eyes

And it’s easy for us to see it as an opportunity for self-promotion. We have an overwhelming urge to dive headlong into an empty diff, riddling it with comments that showcase our own technical mastery, algorithmic ingenuity, and cognitive strength. Surely our teammates will be stupefied by our brilliance. How fortunate they are to be working alongside such a talented engineer!

Even those of us who are well-adjusted enough to show basic humility often fall for a second, less obvious trap. We’re used to having computers follow our instructions exactly. So to us it makes sense to execute our own responsibilities with the same degree of precision. When we’re asked to give our opinion on a chunk of code, we see no reason to hold back. We dutifully identify and mark each and every defect with a tersely-worded declaration of its wrongness. “Another error. Wrong, wrong, wrong. This code is all wrong.” Perhaps we would feel sorry for so vocally calling attention to our coworkers’ failings, but how else will they know what needs fixing? After all, we’re only doing our job.

What Could Go Wrong?

What developers consistently fail to see is that code review is an extremely delicate process. Unlike the direct and deterministic computer processes to which we are accustomed, code review demands empathy, subtlety, and patience. When we treat other humans as if they were computers–without regard for their pride and motivations–the consequences can be dire: severed relationships, escalating retaliation, even a full on FLAME WAR. 🔥🙀🔥

Please don’t make these mistakes. Before you rush in and start reviewing, take a minute to contemplate how others will perceive you. Your coworkers will thank you. And if they’re paying attention, your managers will as well.

Keep Your Eye On The Goal

It seems to me that most of our human code review conflicts arise when we focus too narrowly on the immediate task in front of us and lose sight of the big picture. When we’re caught up in a discussion of the details, it’s common to start seeing the entire pull request as a forum for debate. Naturally, we begin to imagine our teammates as our opponents, to be persuaded of our own viewpoint.

We forget that we’re actually all here for the same purpose: to build software. At the end of the day, it won’t matter who convinced whom of what. All that will matter is what we were able to build together. Although it feels like we’re making progress when we win an argument, the truth in most cases is that the argument didn’t even need to happen. It was, in fact, wasted time.

Any experienced engineer will tell you that time is the most valuable resource when it comes to writing software. So if we’re serious about our job, we must be making every effort to minimize conflict. In every interaction with our fellow engineers, we should be asking ourselves whether our approach really is the path of least resistance. Usually this means conceding points and forfeiting inconsequential preferences. Sometimes it also means allowing our peers to select a design that we know is flawed. As difficult as it may be to accept decisions we disagree with, doing so saves us and everyone we work with the time that it would take to hold an argument. I’ve found that in most cases, these savings vastly exceed any technical cost they might incur.

We Come In Peace

One of the reasons why it’s so difficult to avoid conflict in code review is that the process fundamentally requires us to tell other engineers that they’re wrong. Nobody likes to hear that. Let’s face it: even when it’s constructive, criticism always hurts. That’s why it’s so important to word our review comments with care.

Whenever possible, try to cushion the blow of criticism. If you have to tell someone that they’re wrong–and you do–it helps to throw a few friendly words in there to soften your tone. For example, consider the following comments:

crashes on ios 11

…versus…

hey did you get a chance to run this on ios 11 yet? no worries if you haven’t, I only remembered to because I have a test device sitting next to me. anyway, I’m getting a crash… but I might just be doing something wrong, haha

Now you’re probably thinking to yourself, “That second comment is way too long. It would take forever just to type it out.” Well, you’re right–it does take an extra minute or two to compose it and peck it into the keyboard. But I promise you, the cost of that extra two minutes pales in comparison to the wasted time and lasting interpersonal damage that the first statement can cause if misinterpreted.

Who would misinterpret something so direct? You’d be shocked at just how many of your fellow nerds would. I’d go so far as to say that that majority of disagreements I’ve seen in code review are to some degree rooted in a silly misinterpretation.

The problem with short, direct statements is that it’s easy to read things into them. Perhaps this is the first time you’re working with a new hire, and they have no idea what you think of them yet. In such a scenario, simply stating…

crashes on ios 11

…might come across more like…

CRASHES ON IOS 11!!! WHY DID YOU POST THIS WITHOUT EVEN TESTING IT FIRST?! UGH. >:( >:( >:(

The extra words in an artful comment may seem superfluous, but they’re actually serving a vital purpose: they set a clear tone. The lighthearted tone provides reassurance and diffuses what could otherwise spiral into a tense situation. It says, “Don’t worry, I’m not trying to pick a fight; I’m just doing my job and pointing out something you might have missed. But you’re totally cool! So we don’t have to argue and waste time.”

Invite Criticism

A friendly tone is just as important when you’re submitting a pull request as it is when you’re reviewing one. I’ve found that an excellent way to start out on a friendly note is to welcome criticism as explicitly as possible. In the description for your pull request, you might throw in something to the effect of…

hi peeps, thanks for reviewing my PR! I touched some critical code in the ImportantSystem class and there’s a good chance I botched some of it… so any errors you can point out are much appreciated! also wasn’t sure on some of the conventions, let me know if I’m making a mess :)

You’d think the mere act of posting a pull request would be a clear enough signal that you’re expecting criticism, but reviewers often harbor reservations. Spelling out your posture completely relieves any worry your reviewers might have held that you would seek to argue with them. Lifting this burden of social uncertainty frees them up to concentrate on the code itself. And since they no longer have to tiptoe around your sense of pride, you know that their criticisms will be genuine and complete. That means you can get this pull request merged faster, and it’s less likely that you’ll need to submit a follow-up later.

Avoid Refuting Suggestions

Of course if we’re going to invite criticism, we must also be willing to accept it. As I mentioned earlier, there are two ways to address a comment on code:

1. Accept it. Amend the pull request to incorporate the reviewer’s suggestion.
2. Decline it. Refute the comment with extra information or an explanation of the choice.

One of these options invites conflict. The other keeps us decisively on the path of least resistance. Therefore whenever possible, we must resist the temptation to decline suggestions. Whether we agree with the reviewers’ reasoning or not, being quick to implement their suggestions and slow to dispute them is a solid strategy for saving everyone time.

There could be many reasons why we might not want to implement our colleagues’ suggestions. Chief among them might be laziness. Breaking out the IDE and actually giving a proposal a try generally entails a fair amount of work. In the face of a particularly daunting workload, the prospect of talking our reviewers out of their critique may appear deceptively convenient. But make no mistake: any time we might save is eclipsed by the time we’re about to spend justifying our laziness. Our indignant defense can spur larger arguments, which can lead to lasting animosity and even the total demolition of working relationships.

That’s not to say that there is never a good reason to decline a suggestion. Occasionally reviewers truly don’t know what they’re talking about, and in such cases it’s best to guide them through the misunderstanding with as much courtesy as one can muster. The important thing to keep in mind is that arguing has a cost, and that cost is nearly always beyond what we can afford.

Remember That They’re Just Like You

This may seem like a no-brainer, but don’t forget that your peers are also nerds. Soft skills don’t come naturally to them anymore than they do to you. So if you one of your coworkers leaves a tactless comment or enumerates your errors with all the grace of a compiler log, go easy on them. Wouldn’t you want the same for yourself?

And should you happen to read over a line or two that you appreciate in someone else’s code, be sure to make it known! Offering praise costs you nothing, and it helps keep everyone in good spirits. It’s hard to want to fight with someone who’s sincerely complimenting your work. Recognizing others’ abilities demonstrates that you don’t see code review as a contest to be won, but rather as a team effort. We’re all in this together.

Off To A Good Start

Those are just the basics. These techniques won’t help you dodge every pitfall associated with code review, but they should serve as some rough guidelines to avoid committing the most egregious of nerd offenses. Like most things in life, code review is something you can only really learn through experience. But it certainly helps to approach it with the right mindset. Thanks for reading, and happy reviewing!