So, the reviewer made some comments on a code review and what? Is it clear to your team what happens afterward?
Responding to actionable comments
There are non-actionable and actionable comments. For every actionable comment (bug, suggestion, request for clarification), the developer can respond in three ways.
- ignore it – that’s always an option, but don’t be this guy 😉
- acknowledge it, but postpone the action (for example by leaving a TODO or creating a task)
- do it now – push some new code as a part of the review
Think about the difference between the second and the third option.
It feels obvious that the best time to act is now when everybody is in context. Possible delays with the fix could mean more time spent on getting in the loop. Let’s get it straight – if you put off the action for later, you degrade the quality of the code.
On the other hand, if you fix a thing immediately, you block the code review. Reviewers have to wait for the new code to review it. You risk another context switching if it takes too long.
It doesn’t seem that obvious which option to choose. To help you with deciding, let me bring up the famous 2-Minute Rule.
The rule comes from Getting Things Done and has nothing to do with programming per se. It’s a part of task management system.
If it takes less than two minutes, then do it now.
Simple but powerful. In fact, its magic is way beyond just reducing the number of tasks postponed for later. Immediate sweeping tiny tasks out of your way gives you the clarity in thinking in higher perspective. As David Allen says, If the boat sinks, you don’t think about its course.
Important note: 2-Minute Rule is not about precisely 2 minute time. Rather it means a relatively little amount of time. Less than it would take to track the issue instead of resolving it immediately. Specific time depends on the context. 2 minutes is a safe bet for personal, individual issues. In software engineering, it could be more (or less!).
Feel free to make it “5-Minute Rule” if more convenient.
2-Minute Rule in code reviews
Let me only advertise that I find it surprisingly efficient in real life. Now it will be able to help us with responding to code review dilemmas.
You can establish a rule that if the commonly agreed fix takes less than 2/5/your-call minutes to do, then the dev is supposed to do it immediately.
Of course, there are numerous tasks that take time when implementing “quick” fixes – code, tests, linting, all the git things (commit & push at least), CI loop. Feels to me that there’s not much we can do to spend only irrelevant amount of time on it.
But still, most likely there exist such things. For instance:
- indentation issues
- extracting variables/methods
Those little nasty things tend to pile up to eventually become unmanageable monster called “legacy code”. The more you can get done immediately, the better. Broken windows theory, anyone?
And of course – the rule doesn’t say how to act in case of longer issues. If it’s something substantial, well, you will have to make your reviewers wait for it.
Benefits of this approach
- you don’t degrade the quality of the codebase
- you do block the code review, but only for little time
- when you postpone a thing, you know it’s something worth it
- it gives you a more systematic way of responding to code review comments. This thing is often neglected in the code review process but can help people in recognizing team standards.
- it gives you a clear way to escape pitfalls of tricky tasks
- Hm, it’s not as easy as I thought. 2 minutes are long gone. Let me message my reviewers that it requires a separate task and more time to do
- it’s a metric with all its benefits. It’s easier to discuss with numbers brought to the table:
- You know, this 2-minute rule is useless in our case. Our tests need 10 minutes on the CI, so people are getting distracted anyway.
- This sounds much better a discussion than, say: I won’t fix it just now, because we need to ship it ASAP.
Responding to comments is a part of The Art of Code Review. It shouldn’t be forgotten when establishing a process for code reviews in teams and companies.
Moreover, real-life inspirations can influence and enhance this process. You don’t necessarily have to reinvent the wheel.