We expect every change that gets merged into a maintained project to be reviewed by at least one team member with enough domain knowledge to ensure the correctness of the change.

This rule applies even to new projects that may have a single developer initially responsible for it: if you’re such a person, find a teammate (or two), onboard them, and ask for reviews.

We do not expect reviewers to find bugs in the reviewed code. For that, we have tests. So, while every discovered bug is nice to have, please don’t expect reviewers to be extensions (or worse, alternatives) to a comprehensive test suite.

Instead, reviewers should:

• Ensure that the new code achieves the declared goal. Sometimes a pair of fresh eyes may notice that the solution is either suboptimal or even won’t work in a real-world scenario.
• Agree that the code is maintainable. If reviewers have trouble reading the new code, other people will too. For the code to be merged, it should get to the point where the team is comfortable working with it.
• Absorb the knowledge. Some of the most active codebases may get 10s of PRs merged per day. With such velocity, if new features are added unnoticed, the bus factor may decrease uncontrollably. We always want to have at least two persons who know how (and ideally, why) each particular feature was implemented.

On the other hand, it is the author’s responsibility to provide the reviewers with all the required context (preferably in a form that will remain available to developers after the PR is merged). The reviewer must have access to the goal that the PR aims to achieve. If it is not clear whether the goal has been achieved, the burden of proof lies on the author. Finally, reviewers may expect that if they have a question, they will receive an answer in a reasonable amount of time.

Nitpicks

We consider nitpicking to be perfectly fine.

Our practice shows that the set of nitpicks, if they come from goodwill, is quite limited, and once a new team member adjusts to this set, the amount of comments per PR drops significantly. As a benefit, we have code that follows generally the same style.

At the same time, nitpicks are nice-to-haves, not mandatory to fix. If a comment is marked with ‘nit’ and you don’t really agree with it, you are totally free to politely disagree and resolve the comment without fixing it.

If two team members happen to suggest mutually exclusive nitpicks (though that usually doesn’t happen), please highlight this fact and suggest finding a solution that satisfies everyone: by doing this, you’ll also contribute to aligning the team in the same direction.