Code reviews are important, not only to ensure code quality, but also so project team members can learn from each other. To get the most out of the process, code authors should voice their expectations of a review, and reviewers should state the performed actions and areas of focus.
The necessary prerequisite for a line-by-line review is that code authors make sure that the changes they propose actually make sense and the unit and integration tests pass. It is still okay to request feedback on work in progress, but especially then it is important to be explicit about the depth of the desired review.
As code author (pre-review):
-
Make sure your proposed change does what you think it should do. This also means that it conforms to the code style, tests pass, etc. Do not waste the reviewer's time with things you can easily get right in the first place!
-
Send a PR that follows our PR guideline, and describe what you think are the aspects that require the largest share of a reviewer's attention:
"Please do / look for / look at the following: ...."
-
Suggest the review to someone, e.g. by using GitHub's review request feature, or by leaving a comment on the pull request page, mentioning the desired
@<reviewer>. Don't be afraid to send a reminder after a couple of days if no one has addressed your request.
As code reviewer:
-
State your intent, e.g. by using GitHub's self-assign feature, or by leaving a comment on the pull request page.
-
Perform the review. Try to catch mistakes that might have slipped through automated testing, e.g. if a docs PR adds URLs, check if they actually resolve, etc. Be diligent, but also remember that the goal is not to demonstrate your review skills but to integrate the proposed bug fix or feature. And be respectful in word choice and tone.
-
Mention the areas (topical, regional) you focused on. Feel free to mention areas you cannot review as well:
"Here's what I did to review your code: ...."
As code author (post-review):
-
Address all (!) review comments. Do not waste the reviewer's time by forcing them to leave the same comment twice!
-
Reply to the comment if it is unclear or you think the reviewer got something wrong.
-
Otherwise, push new commits that address the comment. These commits follow the same guidelines as regular commits. Also see notes about rebasing in an open pull request.
-
-
Start over with "As code author (pre-review)" above...