Code Review Patterns
Interrelated issues leading to slow and poor quality feedback
Most developers would agree that getting feedback from peers in the form of Code Review is valuable. A Code Review provides an opportunity for:
Finding Coding Errors
Finding Design and Requirements errors
Sharing knowledge about approaches
A check on understandability of the code and design.
Some of the benefit of review comes from the framing the developer does to set up the review, as explaining yourself is a good way to find errors in your logic.
A useful review is, among other things:
Timely: feedback is relatively quick.
Relevant: The review takes the context of the task into account; the review should not be a place to recap design decisions and conversations.
Actionable: The review comments allow the developer(s) to improve the code.
There are a number of ways to get the feedback from a code review:
An review process using Pull Request tools.
Getting feedback from a colleague in your workspace. This includes Pair Programing.
Each of these approaches creates an environment that makes it easier to attain some of the attributes:
For Pair Programing
The reviewer has all the context, and that it is easier to get timely feedback.
Any workspace based review could make automated checks harder to run, as many tools integrate well in into more centralized review tools. Local to the workspace feedback also doesn’t share knowledge of the change as widely
For a review using Pull Request tools:
Facilitates using Automated Checks for feedback on low level issues, freeing humans to focus on semantic and requirements issues. A PR also makes it easier to share changes with a wider audience.
Since Pull Requests are often asynchronous by default, they lend themselves to longer feedback cycles.
Regardless of the mechanism a team uses for code reviews, the challenges that arise with code review processes are often connected to external forces, and are often interrelated.
Consider Timeliness: When feedback is slow there is a tendency to make each change set larger, which in turn leads to longer review times. And in some cases the change may start out large. Or the team may not prioritize feedback.
The obvious fix to avoiding a cycle to longer code reviews is to work in smaller steps and review code promptly. But there may be other forces making that hard:
The team might not plan work in small enough tasks to make for small review units.
Perhaps the team’s priorities don’t don’t allow for prioritizing reviews.
Maybe no one on the team feels like they have enough context to give good feedback in a short time period.
There may be some simple solutions, but for them to succeed you need to understand the context. And in most cases the more sustainable fix requires looking across the system, and collaborating with others. But there is a lot a development team can do to improve their workflow, and along the way, identify which impediments arise from other parts of the system.
Working in the context of patterns (known good solutions to problems when they appear in a context) is helpful for capturing these dependencies in the problem space.
Even if you want to do away with Pull Request and Branches, the forces, and results of in many of the patterns will still be relevant. You still want feedback to be timely, relevant and actionable.
I’ll share more about the Code Review part of the pattern language next.