Contextual Pull Requests
Understanding the problem can be more important than understanding the technology when it comes to Code Review.
Who participates in your Pull Request Reviews, and do they give valuable feedback? A good review process provides feedback that is relevant, timely, and actionable. In a prior post, I discussed the importance of understanding the reasons for your participation in a Pull Request Review. Being intentional when you do Pull Requests can help you focus on the right things. Being intentional is necessary for effective reviews, but it’s not enough. Ensuring that people giving feedback on code have the appropriate context is important for ensuring that the review adds value in a reasonable time frame.
Who Reviews?
Relevant feedback addresses the code’s context, including requirements, design discussions, etc. General programming feedback about style and related issues is useful, but Automated Checks are often better for that feedback. The more context the reviewer has about the problem, the solution, and the design, the easier it is to provide relevant feedback. Typically, people who are asked (or even required) to participate in reviews are often some combination of:
A designated rotating reviewer
Whoever on the team is available
A Domain Expert or Technical Expert from another team
A Manager or Lead
While anyone with a basic understanding of the codebase, the programming language, and the domain can give feedback, the best feedback comes from someone who understands the problem and the design. Anyone on your team should be able to comment on style, risks, and design patterns, but tools can often do that as well (sometimes even better than a person).
A team member who has not been involved in requirements planning or design discussions could be a good person to give feedback on code clarity — it’s often easy to write code that assumes knowledge that everyone on the team has — but that feedback could also be addressed by someone familiar with the problem but who was not an author of the code. The more valuable feedback in a review tends to focus on the implementation of the design and requirements, and that involves context.
If you even found yourself reiterating design discussions during a review, you know the challenges of asking for feedback from a team member who did not participate in the development process. Also worth considering is
Whether the Code Review is the right time to ask for design feedback. (Maybe you should have had a wider team design discussion before coding?)
The impact on productivity of asking someone who does not have context to do a review,
Good review feedback comes from someone with context about the problem. Having context about the problem also means the process can be quicker.
But What About Risk Management?
One argument for asking certain people (leads, Architects, Managers) to review changes is to mitigate risk. Aside from the obvious issues of these people being potential bottlenecks in the delivery pipeline and the likelihood that they will not have full context, Fosgren and Humble note in Accelerate that such an approval process doesn’t have the impact we believe:
We found that external approvals were negatively correlated with lead time, deployment frequency, and restore time, and had no correlation with change fail rate. In short, approval by an external body (such as a manager or CAB) simply doesn’t work to increase the stability of production systems, measured by the time to restore service and change fail rate.
If you have a subject matter expert who has context for your project, or you are looking to address a particular, generic issue, a subject matter expert’s code review can be appropriate. But in many cases, you probably want to have the expert conversation before you are at the code review stage.
Next Steps?
If you acknowledge that reviewers with more context can give better feedback more quickly, there are a few approaches you can try. Pair Programming, while potentially challenging, offers the benefit of having someone with full context who can provide feedback on the code. There is still a risk that, since a member of a Pair is a co-creator, they may gloss over assumptions, though that can be compensated for.
Another option is to plan work with Feature Partners: pairs of developers working on related tasks, who, while not necessarily pair-programming, are committed to providing feedback on each other’s work and offering feedback on all phases of the process, including design and coding. The pairs need not even be working on the same development stack, as long as each has a passing familiarity with the programming language and tools.
If your team is small, the “anyone on the team who understands the problem” model can yield high-quality feedback. Timeliness and the ability to prioritize reviews are separate issues, which I’ll discuss later.
You’ll more easily realize value from Pull Request reviews if you consider who the right person to review code is.