Stop Nitpicking in Code Reviews
One of the best changes I’ve made at work recently is to stop nitpicking in code reviews.
Nitpicking isn’t about code that is wrong but suboptimal. It’s pointing out a variable name that could use a more appropriate word, a conditional that could be formatted more cleanly, or some minor simplifying of logic. Nits don't result in significantly better code, nor do they educate the developer; they are just tiny changes that are, technically, improvements (if not highly meaningful ones).
I wanted our codebase to be perfect and in my mind these nits were an important part of ensuring that happened. I used to conduct extremely thorough code reviews, filled with comments about architecture and bugs but also so, so many nits. The nits often overwhelmed the other comments; for every major concern I’d raise, there could be a dozen nits.
This practice of mine did not make me the most popular coworker. Far from it; I didn’t realize it at the time but my perfectionism was toxic. At best, it annoyed people; at worst, it upset them, and those feelings lasted far past the code review itself.
A couple years ago, a coworker proposed that we see what happens if we stopped nitpicking altogether. Initially, I pushed back on the proposal: why would I want to allow bad code to get into the codebase? But in the end, we agreed to a one-month experiment.
Much to my surprise, at the end of the month, things were clearly better than before. So much so that we barely even discussed whether to continue the policy; it just sort of became permanent because it was obviously an improvement.
There were two big benefits we experienced:
First, we saw a vastly improved signal-to-noise ratio. Imagine a code review that results in five nits and one critical issue to address. In the hullabaloo of fixing those nits, the critical comment can seem less important or even get overlooked.
Without nits, that one critical comment is the only issue that gets brought up. All attention is paid to it, and rightfully so. What is important gets a signal boost; what’s unimportant gets ignored.
Second, it improved everyone’s relationships with code reviews and those who conduct them. I’ve seen plenty of people talk online about how you shouldn’t take code reviews personally, that it’s the code being critiqued not the person, blah blah blah, it’s a bunch of bullshit. I’ve been going through code reviews for a decade and it still stings when someone points out my mistakes or pushes back on my code designs.
Imagine: you start a pull request and the first reviewer points out a dozen nits. Your natural response will not be zen-like acceptance; it will be to get upset. All you want is to merge this code, but some jerk is making you jump through dumb hoops to do so first. Suppose, now, that you fix the nitpicks and send it back for review, only to get blocked again because the reviewer noticed a few more nitpicks. Argh!
Developers like to imagine that they are composed of nothing but cold, hard logic; but actually, we are humans with unavoidable feelings and emotions.
What was I really doing when I was thoroughly nitpicking code? The main effect wasn’t improving the codebase; it was making people upset with their code and angry with me. They were less receptive to my feedback as a result, and it soured my relationships with coworkers.
A couple years after stopping nits, people are far more receptive to feedback in code reviews and think I’m much less of a jerk. It’s a night and day difference from before.
Ultimately, our codebase has not suffered due to a lack of nitpicking. If anything, it is better than before because we, as a team, work together better and focus on the important parts of code design in our reviews.
But what if you still care about what you nitpicked before? My suggestion is to automate what you’d otherwise nitpick. Add lint checks and code style enforcers to your CI. With tools, you can check for problems in advance; plus, when automation tells you that you’re incorrect, it’s impersonal and as a result somehow less frustrating (like a unit test failing vs. someone calling you out).
But seriously: if you’re like how I used to be (someone who wanted every line of code to be perfect) just try letting go a bit. Check your LOGAF before you comment. Maybe some code will be imperfect, but your codebase and team cohesion will be better for it.