How to create Pull Requests for effective reviews
This is not a definitive guide โ they rarely are ๐ โ but after opening and reviewing thousands of Pull Requests (PRs), I have found these tips to be simple yet effective.
Keep the diff as small as possible
It's difficult to set hard limits for code changes, as they vary depending on the domain. For example, creating a few React components can generate hundreds of lines of code, but they are usually easy to review. On the other hand, understanding complex changes to business logic can be more demanding, so around 100 lines of change may be difficult to review. The rule of thumb is: the fewer lines of changed code, the better the chance of finding bugs and ensuring code quality. Here are some tips to keep Pull Requests (PRs) small and concise:
- Avoid touching unrelated code (e.g. re-formatting or refactoring). It creates noise and can distract the reviewer. It's better to do such changes in separate cleanup PRs.
- Split tasks into multiple smaller PRs. Try to create logical units, such as:
- Splitting a backend task into database/model changes and route handlers
- Splitting a frontend task into routing, data-loading, and styling/UI component PRs.
If you work in a continuous deployment team (congratulations ๐ ), feature flagging allows you to integrate changes incrementally in multiple small PRs.
Add a meaningful PR description
Adding a meaningful PR description is essential to help reviewers do their job effectively. It's not just a bureaucratic act (that code speaks for itself, right?), but rather providing context for the reviewer to evaluate the quality of the code. Taking a few extra minutes to present the changes in a decent way can make a big difference. Consider including screenshots or other visuals to explain changes, especially for frontend PRs.
A possible structure for a meaningful description includes:
- Why: Explain why this change is required and what problem it solves.
- What: Briefly outline which parts of the code and application are affected.
- How to test: Provide instructions on how to test the functionality. Best case: you have automatic review environments for each PR.
- Additional notes: List any caveats, such as specific dependencies, related PRs, or anything that needs special attention.
Using PR templates can be helpful, but keep it concise and generic. Too many sections can make it an actual bureaucratic act.
Be the first to review your own code
It's simple but effective: Take a look at the final diff and you'll likely spot typos, unrelated changes, missing code-comments for complex sections, or any dirty parts that were created during exploration but forgotten to clean up. Fix these issues before asking others to review your code. It's a matter of respect to not waste their time and concentration with obvious issues and unfinished work.
This self-review step has a valuable side-effect: you'll get a sense for the length and complexity of your PR. Is it hard to review? Imagine how someone else who didn't write the code would struggle. Consider adding code comments, improving variable names, or splitting the code into smaller parts.
Offer pair reviews
There are PRs that are complex and hard to review, no matter how hard you try. That's ok. In such cases, pair reviews โ either in person or via video call โ can be beneficial.