Why We Review Pull Requests
- #Best Practices
 - #Engineering Philosophy
 - #Product Development
 
- 2018/12/31 Large feature changes, tiny test tweaks, automation scripts unrelated to the main app—pull requests come in all shapes. Here is how my team handles reviews.
 
Why we review code
Humans make mistakes. Even when the author believes the code is fine, another pair of eyes often finds issues. Reviews are therefore mandatory. They also let junior developers learn from senior developers’ patterns and logic—that is huge. Looking at a teammate’s pull request often teaches me a more concise way to write something. Reviews are not just bug hunts.
Quick refresher: what is a pull request?
If you already know, skip ahead. Folks who have only used SVN in SI projects might wonder.
Most projects now manage source code with Git. Branches let you change code without touching the mainline. Once the change is ready, you must merge it back. A pull request is the request to merge your branch into the main branch. Hosting services such as Bitbucket, GitBucket, or GitHub let you assign reviewers who look over the diffs before merging.
Right-sizing a pull request
Large refactors touch many files and add lots of lines. Stuffing everything into a single pull request may be convenient for the author but painful for reviewers. Keep pull requests as small as possible—ideally one feature plus its tests. Reviewers appreciate thoughtful chunking.
Why spend so much time on reviews?
Reviewing can take longer than writing the code, especially when multiple people need to read it. Why invest that time? Because merging into the main branch often leads straight to production. Even if the author thinks “this is a trivial change—just merge it,” reviewers cannot take it lightly. Discovering issues after release costs far more: you need investigations, impact analysis, and follow-up fixes. That is why owners pay close attention to reviews.
How our team handles ownership
In our team, reviewers approve pull requests but the author presses the merge button. That makes it clear who owns the change. Many teams let the reviewer merge, but by having the reviewee do it we reinforce accountability and encourage an owner’s mindset. Give it a try if it fits your workflow. (We mostly work on infrastructure-related pull requests—automation tools, Ansible playbooks, etc.—so this model suits us well.)