Pull Requests & Reviews
People are far less inclined to "drudge" through a review if they have no clue or context as to what they are reviewing.
Details and descriptions are paramount for PRs that include things like lots of new logic, lots of deletions, and/or new files.
It takes a lot of energy for the reviewer to parse through new code, so making it as seamless and easy as possible by letting them know what you did, why you made certain decisions, etc. is super helpful.
This will certainly require a bit more upfront work on the creator of the PR, but they gain a benefit as well. By forcing oneself to recap and explain their choices when adding in a new piece of software, it increases the author's awareness to their code's readability and maintainability, and may even uncover observations or refactors that would be otherwise overlooked.
Finding Dedicated Reviewers
If possible, one should try and find the last person who touched the files they are updating via git blame or file history on GitHub.
The last person who touched the file likely has a more intimate understanding of it's logic and structure, which will make them a great candidate for a PR reviewer.
Try not to add more than few potential reviewers to each PR. Adding the same 4-8 people to every PR creates fatigue and induces somewhat of a bystander effect.
Make sure to also look at other open PRs on the same repository to check if someone already has been overloaded with requests. Ideally, review requests should be spread out amongst all contributors to a specific repo.
Merging a Pull Request
A pull request needs two approvals before it can be merged into
There are three different states PR reviews can be in: approved, commented, or rejected ("requested changes"). If a PR has any unresolved rejected ("requested changes"), please do not merge it.
The person who created the PR should be the one to merge it, not the final reviewer. This is due to the fact that sometimes the PR creator may be in the process of adding another commit, rebasing onto another branch, squashing commits together, etc. It is impossible to know whether or not the creator is changing the PR, and merging it as a reviewer would mess up this flow if the PR is still in flux.
Once your PR has been approved by at least two people and is free of unresolved
rejections or comments, feel free to merge it into
If you need to make changes to your PR based on feedback, changed features, a found bug, etc., after you add your changes and push to your PR branch, please make sure to let the people reviewing your pull request know that you have modified the code.
Preferably, this should be done with a comment on the PR that tags the reviewer's GitHub handles, not as a Slack message. It is better to keep things related to a PR, including comments and discussions, co-located in the PR itself on GitHub.
Requesting reviews on Slack, or discussing critical details of the PR somewhere other than the PR comment thread makes it hard for someone who needs to reference the PR history (such as another reviewer) to follow important details as to why the code looks the way it does or was updated in a particular way.