The Physical Toll of Code Review

Code review is a common practice most, if not all, software engineers go through. Most of us have had the same, unfortunate experience at least once. It starts when you're in a groove, either through excitement or a general state of flow. You're getting some shit done, writing some slick code, or writing the same kind of code which has shipped countless times before. When you're ready, you double check your tests, look for typos, and then send up the code for a review into whatever version control system you use. You step away, do something else, maybe grab a drink or snack of your choice. All the while expecting to run into little, if any, friction in shipping your code.

And then it feels like someone has pulled the emergency brake.

You see comments on the review which cause a dull pain to cascade through your body or, comparatively, removes your physical ability to keep your face from falling into your hands or onto your desk. You begin to ask questions like, "why?" and also, "huhh??" because the comments or the lack of an approval make little or no sense to you. The code is mostly fine, right? It doesn't break anything and isn't anything new. Why can't I ship this and hit my deadline?

It's a frustrating experience, for sure. Code reviews are one of the few structured engagements where someone is actively looking for feedback or checking with their peers to ensure they are not unintentionally introducing risk or issues to the code. Instead, culture or individuals or poor teaching result in code reviews being fraught with miscommunication, differing expectations, and unclear requests.

It's a great responsibility to be a code reviewer. Reviewers act as a line of defense against bugs, poor architecture, and holding up (or improving) the standards of the code base. It's not hard to make code reviews frictionless and a lovely experience, but it is easy to fuck it up if you're not paying attention to the impact and experience of your peers. Most code reviews are done through tools transmitting at the cadence of an internet forum, so the first place we can fuck up is simply in how we write to each other.

There are many aspects of communication we rely upon which are not obvious when we talk to someone in-person. Consider the difference in how it feels to talk to someone on the phone, or on a video call versus the way it feels to talk to the same person when they're physically in the same space as you.

When you write text, even in response to someone you know well, you are communicating differently and your words take on a different aspect to them. Saying the same thing both in person and in text produces different experiences.

Consider the cues present when you talk to someone in person. There is body language (hand and leg moving, facial responses, how a person sits or stands), vocal tone, conversation pacing, lighting, smells, and more.

Now consider the cues present when talking to someone in textual form. You have the text, the time it was sent, when it was seen, and often not much else. When we lack additional cues, we have to try and fill them in. Reading requires any of us to fill in contextual gaps with what we can discern from other sources.

Similarly, for you as the reviewer, it can be hard to remember this is a conversation when the "someone else" in front of you is a computer screen. You're having to fill in gaps, too. And you may be inclined to have a conversation with yourself as a result. It's hard, as someone trying to communicate an idea to someone else, to speak and wait an unidentified amount of time for a response. We're social and we want a response, even if the response is from ourselves.

This leads to talking about the topic from both sides in a comment, listing out arguments for and against, and coming to a conclusion on one's own. While it can feel maddening, as a reviewer, to wait on someone else to complete a conversation, it's important to only start a thought or respond to one. Having a conversation on your own in a code review will only encourage others to disengage.

Here's a trick if you're feeling stuck: even if you think you know the answer, ask a question! A question is an open conversation by nature. Asking a question also hands the power over to the recipient to make a consideration and take action - either by responding or making some type of change. An added bonus is you may end up with an answer you didn't expect.

This is not to say there are no wrong ways to ask a question. Which leads me to my next point.

The idea of a "stupid" question is disingeuous to the asker. There are many reasons someone may ask a question and most of them are due to ignorance. Ignorance, or the lack of knowledge, is an easily solved problem and is the crux of learning and growing. Asking questions is a great thing to do.

Asking a question because you lack knowledge or context also provides a clear motivation. You want to learn! You want to grow! Your intent is clear! To be honest, I love pausing a conversation briefly with a quick, "sorry, you lost me after [insert point], can you help me understand what this thing is?" The intent of the question is clear. What the asker wants is clear. You know what to respond with even if you have, at first, the impatient response of thinking, "shouldn't this person already know this?!"

What about a question without clear intent?

If the initial response to a question is, "uhhh... what?" or, "what are you talking about? I don't understand," then it may be a question without clear intent. This may feel like an echo of the idea of filling in the gap, so readers don't have to do it themselves. If you're not sure if your question is clear enough, ask yourself: what do you want to happen as a result of this question?

You can ask yourself the same question as a check in for any just about comment in a code review, or really, any communication ever. What is your intent? What are you trying to get across? How much work are you asking the reader to do to figure out what you want? How much are you assuming on their part?

If you don't answer these questions yourself and clarify your intent, then the reader will be asking these questions themselves. Questions like, "what are they really after?" and, "why are they asking such a pointless question?" will be running through their head. Save your reader the trouble and make it as obvious as possible what you want even if you just have to ask, with reasoning, for them to do something.

We should all spend less time trying to translate what we want from each other and more time on solving the problems at hand.

Speaking of solving problems, a code review is about shipping code. Feedback in a code review is important, yes, but it's more important the code is ready to ship. Getting code through code reviews may be kind of important or REALLY important depending on your organization, team, or company culture. In all cases, it is never unimportant. There isn't likely a project or dev manager in the world who would be ok with a code review being held up for something which doesn't have a tangible impact on the function or quality of the code.

The problems we're solving in a code review are few and specific.

• Code is not breaking or prone to bugs
• No significant challenges in architecture or maintainability are being introduced
• No significant risk due to the size, complexity, or embedded logic is being introduced

Beyond the above problems surfacing in a code review, there isn't, arguably, much reason to stop a piece of code from being shipped. Unfortunately, lots of reviewers will hold up a code review for reasons like the following.

• Preferred style isn't being followed
• Something being added could, maybe, at some point in the future become an issue if a specific set of occurrences happened
• Minor suggestions or corrections have been noted

There is no denying some organizations and team cultures in the real world prefer blocking a code review due to a style issue. There is also no denying some teams have a preference to have everything perfect before leaving an approval. We can deny these preferences keep us from shipping faster and having a better experience in shipping code. Imagine a situation where someone has pushed mostly good code with one small trip-up.

In one version of the situation, you catch and comment on the small trip-up, but do not approve the code review. The contributor then has to correct the issue, re-request your review, and you have to spend time going back and validating another developer has corrected the spacing or structure or some other such nonsense. Think about the cost of your time, and attention, and energy. Is this what you wanted to do with your professional life? Double check spacing? No! This is why we have things like linters, or auto formatters, or Continuous Integration (CI) tooling, or automated unit testing, so you don't have to sweat the small stuff.

Now compare the second version where you catch and comment on the same trip-up, and approve the code review. Your teammate will still go and fix it before shipping the code, but you don't have to be bothered again. Your time is free to go deal with real issues. Your teammate now also feels like you've both backed them up by catching a small trip-up and trusted them enough to be an adult and take care of it on their own. Win win win.

Remember, you don't have to be a manager to be a micro-manager.

Code reviews should feel easy to you as a reviewer because they should be one component in a system of components we use to manage complexity, risk, and issues. Each individual component should support the others and therefore be simpler on their own. If reviewing code causes you to feel anxious, try to see if you can manage those feelings by leveraging another tool available to you (or one you can set up).

Other components in a healthy system supporting code reviews are:

• Continuous integration tooling (commonly seen as automated unit or integration testing)
• Live application monitoring
• Application logs
• Simple or low effort rollbacks (alternatively: clear rollback plans)
• Bug or issue tracking systems
• Version control system

For example, I worry very little about shipping bad code or bugs because most, if not all, of the systems I ship code to have automated testing in place as well as easy ways to rollback shipped code. If they didn't have either of those checks, then the complexity would likely fall back on the code review process and increase tension or anxiety around approving code.

Many businesses out there do not have the full suite of supporting systems mentioned above. However, the above recommendations on how to improve communication in a code review are still relevant. If code reviews were our only line of defense against shipping bad code to production, then these tips are collectively here to remind us to chill the fuck out and have a conversation with each other.

Code reviews often break down because we're misdirecting the stress we feel around shipping into our comments and the conversation. We're concerned about the stability of an application because it's critical or had an outage recently or maybe we're just over tired. If you start feeling an overwhelming stress in a review, then step away from the keyboard for a bit and take a break. If you have a real concern afterwards then point to the code, provide your reasoning, and remember your team and the systems around you are there to support the process of shipping code.

You will always be judged more for how you respond to and respect your peers than whether you approved buggy code or not.