Code Review is NOT (just) about Review

Code review is often a contentious point in software development. Fortunately I reviewed for the team for many years now and I gradually adopted below approaches that are seemingly as effective as silver bullet, though I always remind myself before every single review: I mostly likely rely heavily on I have great colleagues that is receiving and assuming good intention.

Alright, let’s start with the catch line:

Code Review is NOT about Review

You got it, I didn’t accidentally added the “NOT”. I meant code review is not about review (defined as “a critical evaluation”). Let’s step back, you want to help your peers, so there are literally two ways: 1. tell them they are great and they can make it, 2. tell them they suck in every single pull request. Which way you want to go?

Many wish they had chosen the first path. It is 10x easier to enhance positive behavior than suppress negative behavior. That means every minute spent on enhancing positive is equivalent to 10 minutes you spent on critique. Thanks for one person I know practicing this everyday and now it is part of everyone in our group.

What this means is when writing a review, try recognize every effort from receiving party, appreciate where things are carefully scrubbed, notice how things are neatly organized, and encourage when it is almost close. Those positive feedback is usually more important than a typo you found.

It is More About HOW you Say it

Engineers are great at Logos but vast amount of the universe don’t work based on logic rules. Especially in communication, we can’t assume the other party will receive your critical feedback like a piece of cake unaltered, but we should rather be aware of the consequence of the words. That is not say what you want to say directly, but say what get the result you want.

For get the result you want, I often clarify why I write this piece of review, such as this is a new pattern that worth extra time scrubbing. Also don’t assume you had the upper hand or the truth, but rather be curious, assume good intention (such as if there is some good reason you don’t know), and avoid accusative sentence. One trick is to ask open ended questions which give people a chance to explain. Here are a few ways to start the question:

1. This is really interesting, I am not super familiar with that block, but can you explain to me why we introduce this object vs reusing the existing one?
2. I see this overwrites a change I made last week. Is there a bug you try to fix for me? Let me know so I can also be aware.

Here is what you don’t want to start your review with for the same situation:

1. Can you reuse the object I designed instead of reinventing the wheel?
2. Can you talk to me before reverting my change?

Ask Questions vs Provide Solutions

Usually, and especially for sensitive situations, it is better to ask clarification questions that lead to one solution the receiver find themselves than provides “your” solution directly. When a solution is provided, it sometimes raise interesting issues such as why your instruction should be followed, what is the motivation behind the instructions. It is better to be sensitive and ask questions that leads to a solution even if that’s not what you have in mind.

There are four type of sentences: 1) open ended sentence begin with what, why, how. 2) close ended sentence begin with “can you” 3) instructions 4) retorts. I personally favored open ended sentence the most. Open ended question provides more freedom for receiver to respond, and they can choose to explain, or provide their solution. Open ended question is least accusative and least offending while retort is completely opposite. Choosing open ended sentence if you can.

It Must be Actionable

Using open ended questions, you should reach a common understanding with the receiver. When you feel you can close it, you need to be crispy clear of what the receiving party can do for you to approve it. It can start with:

1. If you can move this file to that directory, I’d be OK.
2. Sorry, my bad, I still don’t understand. We need to talk. Can you schedule a meeting tomorrow?

Only Request for What Matters

Now, the biggest part is deciding whether you should raise your specific issue after you master the art of how to say it. Here I’m giving high level view and a few caveats. Real world can often be more complicated.

1. Effort vs Benefit

This is a useful chart for first pass analysis. The key is be more cognizant of how much effort you exert on the other party to conform to your idea. It is actually OK to ask for low effort low benefit change. I will think twice if it is high effort and high benefit. I don’t remember I ever asked for high effort and low benefit change.

Avoid High Effort and Low Benefit Review Comments

In the next a few bullets, I explain some caveats to the chart as the world is often more complex than black and white.

2. Reversibility

Pay special attention to hardly reversible changes. Any interface between different services, or between frontend and backend are almost irreversible. Coordinated fix almost never happen.

On the other hand, I’m willing to be more flexible to reversible changes. I don’t refuse to change my code due to the code is reversible as long as the idea is better, but I’d give it a pass if the programmer on the receiving side is not willing to change citing it can easily be reverted if we end up not like the change.

3. Enclosed

What if an issue is not propagated outside of a small piece of code that someone owns and for whatever reason (historically or personally preference for example) they don’t want to change?

In those cases, I’m also more flexible and is willing to give it a pass. There is always infinite amount of difference that you can have with anyone else, and over time your own opinion might change so try your best to accommodate whatever you can.

4. Pattern Setting

There is always the moment that a new pattern emerges. I am willing to spend more time with senior engineers at early stage of a new pattern to allow good pattern to propagate.

On the other hand, for legacy code, I’m much more lenient and will give it a pass especially if the suboptimal pattern I have a problem with is already pervasive.

5. Incremental Improvement

This rule says, if a code is incremental improvement from where current state is, I shall approve. This rule often applies to add a little code into a large code base. Sometimes, endless refactor opportunity can be found in the large old code base, but I’d let go as long as the state is better than before.

Respect Review from Your Peers

To ensure your quality feedback is received, you need to be equally receptive about other’s feedback. Use the same metrics I mentioned to evaluate your response to a review and you will get there.

Getting your Review back Faster

Now we flip the table a bit and exploring what if you have a piece of code to be reviewed. One of the common theme in development is code review takes too long. For this, we identified following tidbits. Nice developers should have already been doing this:

1. During Planning: Cover front end backend interface, schema and major interface
2. During Planning; Plan # of commits and receive feedback in terms of order and whether commits should be smaller early on
3. During Planning: budget time for review, back and forth
4. Prioritize PRs: Create schema change and placeholder class first
5. Prioritize PRs: Ask for review before writing test to make review less noisy and easier to provide feedback.
6. Prioritize PRs: Separate NO-OP, format change into individual PR.
7. Prepare PRs: Insert in PR in Github Images/screenshots/gifs describing what the changes are in Github
8. Prepare PRs: Make comments in Github prior to team review to help keep things clear & understandable
9. Communicate: Clarify timeline and effort required for review on Slack (when is needed, is it just NO-OP, has it pass regression), but keep this really short, 1~2 lines.

Parting Words

We went though the philosophy of writing a code review. As a reviewer, how to assess the strength of the review, and as a reviewee, how to get your review back faster. I hope you enjoy this and learn from it. If you found this helpful, please clap and follow the author.