We all know how to do code reviews, right? We know how the ideal clean code looks like. We have code review checklists, code style guides, best practices and what not. Yet something is wrong. You’ve been working hard for a week on a very interesting task, and became eager to share the results. The code is submitted, but it is waiting for several days before the code review starts. Finally a lot of comments appear. The review gets blocked and you wonder how removing some spaces at the end of the line helps to solve the core problem you were trying to solve. You decide to just do whatever the reviewer asks, wait a couple more days… just to get a bunch of unreasonable and subjective change requests. The discussions in the code review reach a deadlock. Then another striking idea comes up: you have to do a major refactoring along with the solution. The product manager is curious: why does it take too long, and why a team of 2 is slower than a single developer? Meanwhile you have started to work on something else, the prepared work accumulates and you spend time doing branching and rebasing. Coming back to the review becomes annoying and difficult to recollect.
In this situation it is hard to find the way out. Eventually through struggles and pain you have gained experience and became a reviewer yourself. What could you do as a reviewer to prevent this painful frustration of micromanagement in code reviews?
Be the (good) boss
When you are doing a code review for another person you are essentially checking his work for compliance to specifications, standards, best practices of the project, team and company. Usually it leads to a number of advices which hinder an immediate acceptance of the work. So a reviewer participates in the work acceptance process by evaluating the code as good or bad. And this is one of the bosses’ supervision responsibilities: evaluate an employee’s work and provide feedback, both positive and corrective.
Be the (good) boss! Whether you want it or not, reviewing puts you into a position of a supervisory role. Therefore a lot of advice on being a good manager to subordinates applies to reviewers and code reviews. With a computer science education background this skill is not something you learn in school or university. One way to obtain the skill is through code review experience by learning on your own mistakes. But you can jumpstart it by reading up on management topics, and here are several starting point tips related to being the good “boss” to your colleagues during code reviews.
Tips for code reviewers
Split the work
The size of the work to review must be small. This point is connected with the others and it’s important for them to function. If the work chunk is huge, it’s impossible to review it ASAP, it is hard to limit and prioritize the critics. The challenge here is to keep track of what everyone is doing and discussing how a big task could be split into chunks for timely reviews. Leaving a programmer alone for 2 weeks will result in a problematic code review in the end. Some planning ahead is required in order to split the work on a huge task, and some extra effort is needed to make the partial changes coexist and work well with the production code. If this is undesirable, another approach could be to put up the commits for review as soon as they appear without completely finishing and polishing the whole change set.
It is important to give the feedback as soon as possible while the task specification and implementation decisions are still fresh in the author’s mind. An immediate feedback while the author is still “in the flow” is ideal, but not always possible. In general the shorter is the review waiting time the more eager is the discussion and the more effective is the code adjusting process. Likewise if the review process is slow the amount of unreviewed work tends to accumulate until it becomes unbearable. Set up your own standards like: “a code must be reviewed within 24 hours after submission”. Even better is if each reviewer allocates time slots during the work day dedicated to code reviews, because then the review time becomes predictable, and the other party can count on it and plan around it.
In an ideal world all issues must be fixed. As a good boss you understand that it is not possible. Some fixes are “nice to have”, while just a few are “must have”. Being clear and realistic about the next desired step is important. It is not useless to make some remarks as optional TODOs or “FYI”. You act as a teacher uncovering an alternative point of view that might affect the author’s future way of thinking. Some code review tools support a way of prioritizing comments, but otherwise feel free to invent your own ways, for example using colors or emojis. Prioritization is also required for the next step: limiting the number of remarks.
Limit your critics
Code authors are usually human beings. As human beings they have feelings, unconscious reactions and attention diversion. The more critics you add the less focused and diverted becomes the follow up process, and the more it starts to feel as micromanagement. So try to keep the number of problems to fix minimal. If you perceive something as bad or problematic in the code, it doesn’t necessarily need to be reported, at least not in the first round of review when other major points were noticed. As a rule of thumb try to constrain your feedback to a maximum of 3 issues that are absolutely important to be fixed.
See the big picture
As technical specialists we imagine how the ideal clean code should look like, we know what is right or wrong from the technical standpoint considering consistency, readability, performance and so on. While reviewing, try to adjust the clean code ideals from the standpoint of the company’s goals. Is it more important to get the code right, or is it more important to get the task done? Most of the time it’s a combination of both, and often getting the thing done is more important due to the time and budget limitations. So when in doubt about the relative importance of your code review remarks try to put them into a bigger picture.
Empower and trust
The ultimate desire is to be able to trust and rely on the code author’s solid common sense and experience. Making the review comments would not be regarded as an enforcement of your views and implementation approaches, but as a declaration of concerns where you trust the author to use the right technique. Moreover it should become more up to the code author’s judgment to decide what to fix and what not to fix provided with the code review feedback. In some cases the code change is so minor or straightforward that the code review could be bypassed. If the developer has been given enough trust, feels responsibility, the whole development process speeds up, obtains fluency and enjoyment.
What about strict code review cultures?
There are projects that are exceptions of the rules above. For example if a cost of mistake is very high (like in finance, healthcare, mass production etc.), the work requires more scrutiny, more code reviews, less trust. Another example is if the work is done by low-paid, low-skilled programmers. The code review process is different then, perhaps more bureaucratic. But in any case the human nature doesn’t change, same principles apply and you reap what you sow.
Code review is not an impersonal act of commenting on someone’s code. It’s a human dialog between a reviewer and a code author where the reviewer tries to influence the author in order to achieve some common goals.
As a reviewer you can wear a boss’ hat, but you should learn how to be a good boss. Several tactics can help avoiding micromanagement in code reviews: divide the review into smaller individual pieces, provide the review feedback ASAP, make a clear and short prioritized list of found problems such that it is aligned with the project goals. At some point you should start to trust the code author’s decisions and let him be responsible for the right technical solutions.
|Subscribe to get more articles about programming languages|
|Follow @battlmonstr||Become a patron|