The Problem with Async
One way of getting feedback is via a Code Review, often using a Pull Request. People have a lot to complain about when it comes to pull requests: they can be slow and depending on who’s doing the review, give low-quality feedback. The larger issue is that a Pull Request is often done in an asynchronous fashion:
you push change
someone comments
you reply
you wait for a response
etc…
You can address this by making all reviews synchronous. You can do with a Pull Request by simply gathering — physically or virtually — around a shared view of the PR. This is often a good answer, but demanding that all feedback be given that way isn’t perfect.
The Problem with Sync (sometimes)
As I was working on a draft of the Pull Request Pattern, I was reading Slow Productivity, and came across this thought:
A direct strategy for reducing collaboration overhead is to replace asynchronous communication with real-time conversations. ... Arranging these conversations, however, is tricky. There’s a reason why the saying this meeting could have been an email has entrenched itself as a workplace meme in recent years. If every task generates its own meeting, you’ll end up trading a crowded inbox for a calendar crowded with meetings—a fate that is arguably just as dire.
Consider what needs to happen for synchronous review:
Your code needs to be ready
The reviewer needs to stop what they are doing and review your code
You need to be ready to accept, comment, and apply the feedback
All of which means interrupting work.
If you are Pair Programming, this is probably less of an issue since you co-created the code (though some have argued that because you have co-created, there is a risk of tunnel vision that you can avoid by getting feedback from someone familiar with the work, but not an author). Often, a reviewer is a team member familiar with the problem but who may be working on another task, perhaps an adjacent one.
In some cases, the cost of the interrupt might be worth it. But there are some cases where allowing for a buffer between “code is ready” and starting a review makes sense:
When the feedback is trivial to apply, and no discussion is needed (“consider changing this name to be more evocative“).
When there really isn’t anything actionable that came from the review (“This really looks fine”).
When the code is complex enough to need review before someone can comment (“I need to stare at the algorithm and test cases to make sure that all the details are correct”).
When the feedback has some complexity (“refactor this like this example”)
In the first two cases, there doesn’t need to be a meeting. In the the last two, meeting later allows for higher quality collaboration time. And in the last case, it could be that there really isn’t a conversation needed at all. One can even imagine a scenario where a potential merge has a risk score associated with it, and changes below the threshold, which pass Automated Checks, don’t need a review (though an author can always request one because collaboration is a chance to learn and help).
Structure
There are a few ways to minimize the overhead of review time, and each team can have a different approach. Some options that can work are:
Schedule Review Windows, 2 or 3 times a day when developers prioritize reviews.
Have a process where you announce that the code is ready and expect that there will be some feedback within a window (maybe an hour or two), including whether a synch review is needed.
Conclusion
Poorly done Pull Requests can slow teams down significantly. If you can get immediate feedback on your changes, and it works for you and your team, you should. But for many teams, having a process that allows for a (short) review prep window can make collaboration time more effective and, in some cases, eliminate it for trivial changes. Collaboration is good. But meeting for the sake of process isn’t.
Nothing I’m saying here takes away from the fact that Code Review feedback should be Timely, Relevant, and Actionable, and that for that to happen, reviewers need to have a “Feature Partner” mindset, being familiar with the work, and able to prioritize reviews.
—
Note: links to patterns are to work in progress drafts. Comments are welcome.