A Guide to Mindful Communication in Code Reviews – Kickstarter Engineering


Code reviews are a core part of being a software engineer. Most of us review code every day. Technical skills are needed to review code thoroughly and quickly. But beyond that, reviewing code is also a form of communication, teaching, and learning. Either as the author or the reviewer of code, being mindful in your communications can make code reviews more valuable for everyone.

Mindful communication

First of all, what does it mean to be mindful in your communication? Mindful communication is a term that means to listen and speak with compassion, kindness, and awareness of yourself and others. This perspective is the basis of most of the tips in this guide, so let’s dive into it a bit more.

Here are some general guidelines for practicing mindful communication:

  • Put yourself in the other person’s shoes.
  • Listen to hear what’s actually being said.
  • Let go of what you think the outcome should be.

These things are easier said than done! So I’ll break down how these guidelines apply specifically to reviewing code and having your code reviewed.

Tips for everyone

A crucial skill to learn for practicing mindful communication is a self check-in. This means taking a moment to see how you’re feeling and take a step back from what you’re communicating.

When you’re doing code reviews, you’ll want to check in with yourself to see if you’re following the guidelines. Good times to check in are right before you start or reply to a review, if you’re feeling frustrated, or right before you hit “submit” on that comment.

When you’re checking in, also consider how you’re feeling in general. Giving kind and considered feedback takes time and energy. If you’re hungry, tired, in a hurry, have a lot of meetings, etc., don’t review code or send out a review. You need to fix those things first. If you don’t care for yourself, you can’t take care of others.

Here are some good questions to ask yourself when you check in:

  • Why did this person write this code the way they did?
  • Is the tone and wording of my feedback something I would feel good hearing?
  • Do I fully understand where they’re coming from?
  • Do I think I know the “right” way to write this code? Am I allowing space for a solution I haven’t thought of?
  • Do I have all the information I need to understand this issue?
  • Have I eaten lunch?
  • Do I have enough time right now to give thoughtful feedback?
  • Am I feeling upset/nervous/anxious about something else?

Tips for the reviewer

1. Don’t jump to conclusions

Assume the author did what they did for a reason. Put yourself in their shoes. When you make a code change you likely consider a few options, and go with one that fits the needs and constraints of the problem you’re solving. They likely did the same thing.

Instead of telling the author they did something wrong, try:

  • “What about this other option? It might make xyz better!”
  • “What happens in abc situation?”

This also hits on letting go of the outcome. You might not have the right answer. You might not have all the information you need to understand why the author took the approach they did. If you step back and try to understand where they’re coming from, it might surprise you!

2. Ask open questions

Listen! Don’t assume. If you think something is 100% off the wall, check in with yourself. You respect your co-workers, so ask them to explain their thinking. You might learn something, or get a glimpse into someone else’s thought process.

Try these formats:

  • “This part is a little confusing. Could you walk me through what it’s doing?”
  • “I’m having a hard time following this section. How does it work?”
  • “What other options did you consider for this part?”

3. Leave out the nits

There’s no need to call out nits or small things that maybe aren’t your style. Linters are great for catching small issues or keeping a code base consistent, so there’s no need for a human to call those sorts of things out. If a linter could catch what you’re about to point out (even if your current setup doesn’t), think hard about whether it’s worth everyone’s time to discuss.

Labeling a comment a “nit” is also to be avoided because it minimizes your feedback. Your input is valuable, so leave out the caveats. If you don’t feel your comment is worth adding without a caveat, consider leaving it out completely.

Avoid starting comments with these:

  • “Nit: …”
  • “This is a nitpick …”
  • “Tiny nit: …”
  • “This is slightly nit-picky, but…”

(All pulled from real PR comments 😱)

4. Call out if things could be fixed later

So you’ve left out the nits, but there are still things you think could be improved that aren’t strictly necessary. Let the author know you don’t mind if they’re updated in a later or follow-up PR. You might not be aware of a deadline for the team or that there are a lot of changes still coming.

Here are some phrases that acknowledge iterative work:

  • “This could cause x problem later, but I think it’s fine to fix in the next iteration”
  • “Can we make a ticket to come back to this section? It might cause x issue down the road!”
  • “Have you thought about how this will generalize to what we’re working on next?”
  • “How are you planning to handle this other situation that’s coming up?”

5. Be biased towards approving

Our work is iterative and subjective so don’t be a gatekeeper. Try to approve reviews even if you disagree with the style or approach. People write code differently, and come from different perspectives.

Though I advocate for not blocking PRs on reviews, often the author must get your approval before moving forward. This creates a power imbalance. Avoid abusing that power and approve it unless you’re worried a change will cause major problems. If you’re that worried about something, another option is to go talk in person or use some of the open questions from my earlier tips.

An exception to this is if the change is very large or includes an architectural decision. When a decision is being made that will have long-term consequences, it likely warrants a larger conversation than a code review. After that conversation the review will likely be a quick 👍.

6. Give example code

Particularly when reviewing code for someone more junior, giving a specific example in the review comment can be helpful. This saves them time, and makes it really clear what you’re proposing. Also, you already thought through it, so why make them figure it out on their own?

Here are some examples:

  • “This section might be more clear if you switch the order like this:
    if (true) {
     return “this is an example”;
    }”
  • “You might be able to replace this part with a utility we have! I think this would do the trick: const x = utily_thing(y, { a: true })”

7. Link to documentation

Linking to documentation or files you referenced shows the author where to find the information next time. This is particularly good to do if you looked it up while writing the comment. You already have the link, so you might as well. Sharing is caring! But acknowledging that you needed to look something up can also help fight imposter syndrome.

Calling out that you learned something new in the review can have a similar effect. If you come across a new method or syntax that you need to look up, leave a “TIL” comment and include the link that explains what was new to you.

8. End with “What do you think?”

Ultimately the author of the code will be making and owning the change. If you have a more fundamental or big piece of feedback, end the comment with “What do you think?”

This goes back again to letting go of the outcome. Maybe you don’t have all the context — maybe the author already thought about the approach you’re suggesting and there are issues with it that you missed.

You can also ask for history about how the decision was made. If you weren’t part of conversations about the approach, getting some history can be helpful before suggesting a different one. Sometimes junior engineers are following the advice of another more senior engineer, so if you disagree it likely makes more sense to talk to the senior engineer.

Hopefully large or fundamental changes aren’t needed by the time code is reviewed, which leads me into tips for the author.

Tips for the author

If you review code, you likely have your code reviewed also, so think about what makes a PR easier or harder to review.

1. Make small iterative changes

It is so much easier to review small changes — the smaller the better. Smaller changes are less risky, and take less time to review. Reviews with many hundreds of lines changed take much longer to review than several small ones with isolated changes. If each PR maps to one piece of work, it’s easier to understand what the code is doing. The reviewer doesn’t have to pull apart which changes are needed for each task.

2. Link to documentation

Sound familiar? This is a tip for both reviewers and authors! Linking to documentation in your review description or comments can make it easier for the reviewer to understand what’s being changed or implemented. If you’re implementing an interface or using a new library, link to the documentation you referenced while doing your work.

This is particularly helpful for seemingly small changes in things like configuration. Have you ever received a one-line change and had no idea what it meant? A link to the documentation is so helpful in these cases.

3. Assume the reviewer has the best intent

Even if a reviewer follows all the tips above, they can still make mistakes or get careless. Try to keep in mind that we’re on the same team and they likely had good intentions behind whatever feedback they gave. It’s easy to get defensive, but remember, as the author you have to let go of the outcome just like the reviewer. Maybe the reviewer has some knowledge or expertise you were missing, or saw a blind spot you missed because they came from a different perspective.

A major exception to this is if the reviewer legitimately doesn’t have the best intent. If someone is being actively malicious or hostile in a code review, that’s harassment and you should speak to your manager and HR about it.

4. Think about the readability of your diff

Your diff itself is a kind of communication. Make sure it’s well written!

Here are some things to think about when cleaning up a diff:

  • Don’t refactor in addition to new changes.
  • Check that your PR doesn’t have unnecessary changes or unrelated files.
  • As mentioned above, each PR should be one logical unit of work.
  • If you find yourself leaving explanatory comments on your own code review, those might be better as code comments.

Sometimes diffs are more easily viewed with no white space. You can see a diff without whitespace changes on GitHub by checking “Hide whitespace changes” under “Diff settings” or by adding w=1 to the url. On the command line you can use git diff -w. You can call out that a particular change might be better to view that way in your PR description.

5. Get feedback early and often

Having a quick chat about some implementation options with someone who’ll review your code is a great way to avoid stress when the review comes around. Reviews are usually not the time to work out architectural decisions since the work has already been done.

This sounds like hard work

If this all sound like a lot of hard work, that’s because it is. Like I said earlier, giving kind and considered feedback takes time and energy. But luckily it gets easier.

You can build up muscle memory of actively giving kind feedback and it will become natural. At first it might feel uncomfortable to ask a question when you think you can just tell someone the answer, or it might feel slow to write out a code example. You might have to stop, check in with yourself, and do it anyway. This is a learning process and learning new things is uncomfortable.

Lastly, keep in mind that every team is different, so these specific tips might not all apply to your team. If you come up with other ways to apply mindful communication to code reviews or building software in general, I’d love to hear about them!

Thanks for reading

This post is adapted from an internal talk I gave a while back. At our biweekly Engineering team meetings, anyone on the team who has something to share can give a talk. The talks cover everything from explainers about parts of our infrastructure, to a new Javascript pattern, to how teams talk about their feelings. We also start each meeting with an “opening act” that’s usually someone from the team playing a little guitar or showing off some weird internet art.

If you enjoyed this post, come work with me! We’re looking to add a whole bunch of folks to the team.

Thanks to Natacha, Mark, Janel, and Corey for reviewing this post. ❤️



Source link