Humane code reviews
- Publish Date
- Steve Jackson
- Sam Jones
Most software teams we work with have already adopted a pull request review process. The act of reviewing code seems simple, but it’s often time consuming. A deliberate approach is needed for a team to realize the impact of investing their time in pull request reviews. A good review process fosters leadership, knowledge sharing, collaboration, and more humane communication practices.
When we’re joining a team, we can often see the structure of influence by analyzing their pull request review habits. The opinions of leaders are sought after, and their direction defines the technical solutions a team develops. However, leadership is actually shown by all members on a team in a healthy review process.
Leadership is demonstrated by authors in the way they request reviews and by who they are bringing into the conversations. On healthy teams, reviewers are deliberately chosen for reasons like:
Rachel, you understand ActiveRecord deeply, can you check these scopes to see if they can be improved?
Hung, I think this will help with that incident we worked on Thursday. Is there anything else we could add?
Justin, this is an example of how we want to test our GQL components. Does this translate easily to what you’ve been working on?
Each of these examples shows the author requesting a review for the purpose of growth and improvement, not just delivering a bit of code. Good leaders are able to prioritize the humans involved, using pull requests as a medium for communication and collaboration.
When reviewing code, our primary responsibility is to the author and their growth as developer. Spending time on code reviews is a large investment, and focusing on just the code limits a reviewer’s impact to a single change. Often, taking the time to teach pays off in a way that writing the same comment over and over will not. We’ve also found the best way to teach is through pair programming, and asking to discuss a component of a pull request is a great way to build relationships and encourage the practice.
I have a question about a decision in this PR. Can we pair?
Additionally, large organizations often struggle with onboarding and education. Reviews are a great place to teach and explain the context for shared conventions. When we take the time to explain coding patterns to a new developer it benefits the entire team. A reviewer with this mindset is advancing shared understanding, not just enforcing rules.
Leaders understand that good feedback processes improve the health of a team. Pull request reviews give team members an opportunity to practice giving humane, constructive feedback. We’ve found that we can increase the stickiness of code review feedback when it’s framed as a progressive challenge. Call out things done well and reinforce the notion that everyone is on a journey, and you’re only seeing one stop along the way. We don’t just present our idea of the best solution, we help an author improve theirs.
A pull request is a moment in time, and code is malleable. Things don’t have to be perfect right away. It’s more important to empower a team to evolve a solution over time, and grow their abilities to do so. We want to avoid aggressively gate-keeping and withholding approvals.
Ideally reviewers should be extending trust to authors, giving them room to interpret feedback, and allowing them the final decision. This can be difficult for reviewers who feel more responsibility to the application than their peers. It’s hard to stop your contribution at graciously informing the author of the potential effects to quality when they have alternative viewpoints. One useful technique is to categorize your feedback so that it is clear to the author what is critical, and what falls under the category of a non-blocking “nitpick”. On a large team, it may even be helpful to culturalize this practice with some fun emojis 💚.
Most importantly, though, we try to remain gracious. We remember that just because we see something we’d do differently, we don’t need to request that it be changed.
As Neal Lindsay says:
Sometimes a grain of sand in your shoe isn’t a big enough problem to take off the entire shoe.
When we review pull requests, there are many kinds of feedback that we can give. It can be useful to make multiple passes through the review. With each pass, imagine yourself wearing a different hat, using this alternative viewpoint to focus your attention.
There are so many hats you can wear, here’s a list with some of our favorites to get you started.
- 👷 Safety hat: Will merging this code break anything?
- 🧑🏫 Teacher’s hat: Are there opportunities to share alternative designs, established customs, or to promote consistency?
- 🥳 Party hat: Can I celebrate the author in this PR?
- 👨🎨 Critic’s hat: Should I comment on subjective things like names, aesthetics, architectural design?
- 🗣 Conversationalist’s hat: Is there a conversation that should be documented publicly for the team?
- 👩⚖️ Decider’s hat: Do I need to push back on a suggestion or make a decision about the approach being taken?
- 🔎 Inspector’s hat: Will future maintainers enjoy occupying this area of the codebase?
- 🧑🔬 Scientist’s hat: Are there any performance hypotheses that should be tested before releasing this code?
More important than what you will say, is how you will listen to the author. In conversation, active listening is the practice of listening to understand rather than to respond. Growing this skill is vital to increasing your influence, but it can be difficult to practice in a high-bandwidth conversation. Luckily, active listening makes for better reviews. The asynchronous, text based, nature of a pull request gives you more time to understand, and carefully consider your response.
The 3 levels of listening provide a framework for thinking about your listening habits.
- Internal Listening: Performing a review with the intention of showcasing your abilities and knowledge. Enforcing with the goal of protecting the codebase.
- Focused Listening: Asking questions to understand the decisions made by an author through code review. Guiding the author to a better solution based on a common goal.
- Global Listening: Investing time during a review to meet the author where they are and using the review to promote collaboration on the team. Balancing the needs of the codebase with the growth of the author and others on the team.
Intention vs impact
We’ve written before about the importance of understanding the impact in how you communicate in tech. Interactions in a pull request can change our relationships with team members. All participants must be conscious of the impact they have when delivering feedback. Feedback that elicits the author’s intention is going to be more useful. Enabling growth is tricky to get right and requires some practice. However, it will develop empathy on a team.
When asking questions in a pull request, try to understand the author. Let yourself be genuinely curious rather than make assumptions. Ask questions that support the evolution of the author. Let go of attachment to the sanctity of the codebase.
At Test Double, we like to define success by the growth of the teams we work with, as opposed to the presentation of our own knowledge. We believe team members should take pride in the code they deliver, and that they do their best given the constraints they face. We honor their effort and understand that communicating through text, especially with unfamiliar teammates, can alter a relationship with the humans we work with.