4.4.2 Review guidelines for reviewers

Code reviews can be challenging at times. Follow these suggestions when reviewing code to avoid common pitfalls:

Ask questions: What is the purpose of this change? If this requirement changes, what else would have to change? How could we make this more maintainable?
Discuss in person for more detailed points: Online comments are useful for focused technical questions. In the case of more detailed discussions, it will often be more productive to discuss the matter in person.
Explain your reasons: Sometimes it is best to both ask if there is a better alternative and at the same time justify why a problem in the code is worth fixing. Criticism with no explanation can appear confrontational.
Make it about the code: It is easy to take notes from code reviews personally, especially if we take pride in our work. It is better to have a discussion about the code than about the developer.
Indicate the importance of fixes: When offering many suggestions at once, highlight the point that not all of them need to be acted upon at once. Indicate the importance of your suggestions. This lets developers improve their work incrementally.
Take the developer’s opinion into consideration: Imposing a particular design choice based on personal preferences and with no real explanation will incentivize the developer to be passive instead of active and creative.
Do not re-write, remove, or re-do all the work: It sometimes appears easier to re-do the work yourself, which means discarding the developer’s work. This can give the impression that the developer’s work is worthless. It also creates additional work for the reviewer, who is effectively taking on responsibility for the code.
Consider the person you are reviewing: Developers are human beings. Consider their personality and experience when reviewing their code.
Avoid confrontational and authoritative language: The way we communicate has an impact on the receiver. Consider these two statements which communicate a problem in the code: The statement “This operation is wrong, please fix it.” is confrontational and authoritative. Instead, explain the specific error and ask the developer to review the code again.