Today's post is on something of a touchy subject. Most people who become software engineers, myself included, seem to have a bit of a perfectionist streak. We like our tabs to be precisely two spaces (or four) — so much so we reinforce this by setting up VS Code to insert spaces when we tab! I, at least, tend to like short ternaries because seeing that logic inlined makes me feel some kind of way. I love short circuiting in JSX a la condition && (<Component />).

With that perfectionist worldview in mind, criticism of my work can sometimes be tough to accept. This is particularly true when the criticism A: seems like it's "nit-picking" or B: takes place on code that has to be shipped with some amount of urgency.

At a certain point, what's the purpose of a code review? It almost seems the feedback I give and receive inevitably falls into two categories: nit-picking or non-existent.

This graphic (and the thread from which it came) shows some of the attitude to which I'm referring:

I know it's a meme, but memes tend to have a grain of truth at their core. Are we really suggesting in a field dedicated to continual growth and constant improvement we think the "Lawful Good" option is to provide nothing but a "Commit This" button? There's no usefulness in taking the time to help our fellow devs improve through thoughtful and intentional feedback? It's on the "Evil" end of the spectrum to reject a PR?

I think deep down we can all attest this rings true in many scenarios.

I know it's not always the case, but when we're in a rush to get code out the door or when it's a linter-style nitpick? It's not only that the code is my baby. I swear it isn't just that slander toward the code will be viewed as slander toward me. I'm not too proud to listen and learn...

Or am I?

The truth about code reviews

The biggest improvements I've seen in how I think about architecture and code quality have come during drive-by code reviews provided by senior engineers on our team. Sure, sometimes people DO nitpick. Absolutely, some of the feedback is out of scope or out of place or is worded in a way that's not helpful.

But you know something else? Despite their other shortcomings, it was left by someone who cares about code quality.

And if it's not, I have news for you: you're not on the right team. It's a conversation for another day, but if you don't work with people you trust to have a high degree of "give a shit" about the codebase you're responsible for, that's a separate problem from the cadence of a code review.

If we all care about producing good things — and I think we do — how can we improve the style and substance of our code reviews to make them as helpful as possible?

Here are a few quick guidelines I try to stick to. Of course if you're my teammate you'll know I fail at this more often than not! It's not about being perfect (in our code or our code reviews) — it's about being willing to improve.

Guidelines to a good code review

1. Don't be a human linter.
   • I wish this went without saying, but things like tabs and spaces, indentation, spacing around brackets, etc. should all be controlled by an automated tool like ESLint.
   • The tools are there; the config is easy. Use them!
2. Know your audience.
   • The goal is not to communicate how YOU would want someone to communicate. The goal is to communicate how THIS person wants someone to communicate. People are different!
   • You should be open to backing off or modifying your approach if you're given feedback they aren't finding your style of review helpful.
   • The substance might not change, but the presentation can (and should!) vary from person to person.
   • Consider: is your goal to help your team and teammate improve? Or is it to be some arbitrary gatekeeper for the codebase, tirelessly defending main from lesser developers' efforts?
3. Be clear which of your comments are truly blockers.
   • If you leave some comments as style suggestions and others that are truly areas where you see the chance for a bug to slip in, the owner of the PR should be able to distinguish between the two.
   • Our team has been using emojis to serve this purpose, and I've really liked it.
4. Take advantage of built-in reviewing tools.
   • If you see a Boolean that says && but should say ||, there's a button right in the comment to Suggest Changes!
   • Making your small critiques as simple as pushing a button will lend more weight when you have suggest a larger rework.
5. Take your time.
   • I think this is probably the most important — and underrated — rule for a good code review.
   • If I'm not willing to spend the time understanding what's going on in the code, why should the submitter take the time to listen to my feedback?
   • If I can't make myself available to help them understand my suggestions if they're unclear, why should they respect what I'm saying?
6. Submit your PRs as though you want feedback.
   • From the perspective of receiving a review, double check your own pull requests to see if they invite conversation.
   • Are you providing adequate background information on business rules or implications about your code in the PR description?
   • Could another developer unfamiliar with your recent work — but experienced in architecture, for example — come into your PR and reasonably expect to understand what's happening?
   • If you aren't inviting others to comment on your work, consider most people view this as a two-way street. Be open to criticism before giving criticism!

I fully understand it's a commitment to divert attention from our WakaTime numbers and endless meetings to conduct a good code review, but I believe it's worthwhile. This is all about having discipline. Similar to writing documentation, if we establish good patterns to encourage thorough and thoughtful reviews, that cadence will be easier to maintain when things get hectic. And contrary to popular belief, sticking to this process when things ARE chaotic will keep the chaos in perspective.