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 […]
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.
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:
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.
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:
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:
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!
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:
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:
(All pulled from real PR comments 😱)
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:
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 👍.
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:
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.
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.
If you review code, you likely have your code reviewed also, so think about what makes a PR easier or harder to review.
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.
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.
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.
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:
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.
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.
If this all sounds 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!
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. ❤️
A Guide to Mindful Communication in Code Reviews was originally published in Kickstarter Engineering on Medium, where people are continuing the conversation by highlighting and responding to this story.