Reviewing the Code Review (2 Part Series)
In Part I, we discussed specific suggestions to improve code reviews using PRs, and how the methodology itself has inherent limitations. We asked:
- What if you did not need a PR to start the review?
- What if you could just tag and comment on any part of the code at any time?
- What if you could link two otherwise unrelated parts of the code to highlight a dependency or illustrate an approach?
In this post we review the suggestions made by Jonathan Maltz of Yelp in his article Code Review Guidelines. They are similar to many other articles you can find on the topic, so they serve as a clear outline for this discussion.
Jonathan points to general themes that tend to improve code reviews. These are some of his key messages:
- Communication is key
- Smaller is better
- Make your code easy to review
- Code reviews are a discussion, not a dictation
- Time is of the Essence
- Review for Correctness
Note that there is an overarching theme here that you can read between the lines. The PR is in fact determining the cadence. We are used to PRs defining what is possible. I will now suggest that there may be a better way to frame code reviews in the context of a continuous approach.
At CodeStream, we are building the tools that allow code reviews of any portion of your code at any time without the need for a PR. We call that approach a Continuous Code Review and it is ideally suited to the DevOps era. Let's look at Jonathan's suggestions in the context of this mindset.
Of course, communication is key when reviewing code, since the only way to clarify issues and reach alignment is to communicate. But how are you communicating? Are you using Slack or Microsoft Teams, cutting and pasting code snippets and hoping to clarify matters outside the context of the IDE? Why not avoid context switching altogether and stay in your editor to communicate? That would simplify and improve the process.
Smaller is better is also a way of saying that small chunks are easier to understand, process and learn from. Why not make it as small as possible? What if you could just discuss and review any code block at any time without a PR? That would increase effectiveness and provide guidance without any of the trappings of the PR formality.
Make your code easy to review means providing guidance to the reviewer in advance. While that is desirable, it becomes less important when communication is easy in context, If your reviewer can ask or comment on your code and you can reply quickly and without context switching, you will not have to guess what the reviewer might be unclear about.
Code reviews are a discussion, not a dictation means that the reviewer is providing feedback, which may just be a need for clarification instead of a suggested change. But a good and efficient discussion requires great communication, and great communication about code cannot happen with a lot of context switching. See above.
Time is of the Essence refers to turning around the review to the submitter as soon as possible. The suggestion is no more than 24 hours to maintain momentum. Sure, there are many reasons why a review could be delayed, but generally, it's because the amount of code to review is too overwhelming to just knock it out in a few minutes. In a Continuous Code Review context, reviewer and reviewee would work though small chunks quickly so as to find resolution and clarity and maintain momentum. Issues can sometimes be resolved in minutes, not days.
Review for Correctness means making sure the code is bug-free and solves the problem at hand. But what if you could make suggestions about any part of the code that do not need to be resolved immediately and allow the code to be merged while also providing a mechanism to track areas of concern or improvement right along the code so that anyone who works on it can be informed of those concerns and address them appropriately?
The Continuous Code Review mindset implies a new set of assumptions regarding what to review, where to best communicate during the review, and when to do it. Here are the takeaways.
- As you encounter code that might be improved, it's best to flag it right away. Not limited to whichever PRs might be open, but any code, at any time, on any branch, anywhere.
- For this approach to be practical, it's assumed that you will be in your IDE, where talking about code is easiest, and that a PR is not needed. Otherwise, the friction just grinds the process to a halt.
- Lastly, reviews should happen very frequently. As you work with the code, if you have a question, just ask it; if you see a problem, flag it; if you have a suggestion, describe it; If you have the answer to a question, offer it.
CCR represents an order-of-magnitude reduction of friction for code reviews. By allowing you to review, or ask for a review of any code in your codebase without a PR, and without leaving your editor, you have the opportunity to radically improve the code review process.