Code review is one of the most common practices in software development. Any company I have worked for since 2012 had it as a required step of the development process. Despite its ubiquitousness, understanding of it differed from company to company and even engineer to engineer.
There are more than enough articles about best code review practices, but I'd like to summarize my thoughts and experience in this post.
Define "code review"
According to Google's code review developer guide, "A code review is a process where someone other than the author(s) of a piece of code examines that code."
I would also add to that definition some goal setting:
Code review is a collaborative act between the author and reviewer aimed at delivering the code increment in the best quality and most suitable for further maintenance at a given time budget.
The main output of code review is the improved code quality of the particular change, highlights of issues the author(s) of the change could have missed, and alignment on the balance of trade-offs made in the increment.
Before formulating review principles, let's discuss the requirements for the review request itself:
Creating a PR
Keep Pull Requests small (ideally under 400 lines), and prefer several small PRs over one big (with sensible exceptions, i.e., for refactorings, automated code changes, etc.). Small PRs are faster to review, and reviewers are more likely to find issues (see "LGTM syndrome").
Separate refactoring and actual business logic change into different PRs. Considering that refactoring can be extensive and unrelated to the business logic change, having both in one PR will make it harder to focus on each.
Before requesting the review, make sure all automated checks pass (linters, static analysis tools, automated tests, etc.).
Before requesting the review, look through the pull request yourself for obvious improvements (leftover debug code, accidental changes, etc.).
Format your pull request's title and description according to guidelines, don't leave an empty PR description template.
Add comments explaining certain decisions in the code or providing context (link to documentation, discussion happened elsewhere, etc.)
Communication style in code reviews
Always be respectful, friendly, and professional to your co-workers, and preserve a positive and constructive communication style.
Keep your comments concise. If more than one paragraph of text is needed, leave a note but communicate details verbally (in a shot call or over the desk).
Be explicit about whether the change you suggest is strongly needed or nice to have.
Provide reasons for changes with possible examples, links to guidelines, or best practices when requesting a change.
Code review principles
Technical facts, company dev guidelines, and business requirements supersede personal opinions and preferences.
Respect the task scope. The "boy scout rule" is great and shall be used when there is an opportunity, but be mindful about not creating a significant delay to the task delivery. If necessary, create follow-up tasks for addressing valuable comments that can't be fixed now.
Allow a variety of solutions for edge cases. To support the pace, consider handling missing cases with graceful errors. Iterate within a task or create follow-up tasks if necessary.
Done is better than perfect. Delivered code has more value than infinitely polished.
Roles of team members and outside reviewers
It may happen that a person from outside of the team can add their review to the code even when not requested explicitly. Generally, it is a great practice providing transparency, knowledge exchange, and innovation.
Everyone is free to drop by the code review and add comments, but only the team members make the decision about merging.
This principle is crucial for supporting the culture of team autonomy, ownership, and a spirit of healthy compromise. Outside reviewers often don't observe other factors in balancing trade-offs, so they cannot make an informed decision about the approval.
What code review is not
It is also essential to understand what code review isn't.
Code review is not an attempt to make the code absolutely perfect. Without a limiting factor to balance participants' strive for perfection, review and iterative fixes can take forever, reducing the value of such increment.
Code review is not a competition of who can write more comments on the code change or who knows more ways to implement something. It may feel natural to write many comments and get into the "I'd rather do it in another way" trap. But it is crucial to understand if implementing the comment improves the code.
Code review is not the act of gatekeeping the delivery by some authoritative person or a group. This approach, first of all, doesn't scale well. But also contradicts ownership and autonomy principles while possibly also working against innovation.
A small disclaimer
The principles formulated above are crafted for commercial product development. Some of them may be debatable in the case of open-source development, where effort and time to market are less pressuring (which is also debatable) factors.
