Code review best practices: How to complete a pull request
December 12, 2023 3 min read
Reviewing pull requests (PRs) is a fundamental part of a developer’s day-to-day. Throughout your technology career, reviewing PRs is an opportunity for PR authors and reviewers to collaborate, learn, and grow while building a resilient code base.
Four steps to thoroughly review a pull request
My experience at Capital One has exposed me to various PR review norms. Through the Technology Internship Program (TIP) and Technology Development Program (TDP), I had the opportunity to see how different teams and engineers approach PR reviews. From these experiences, I have crafted a 4 step process I follow when reviewing PRs.
The 4 step PR review process
Understand the high-level purpose
Review the code
Test the changes
Consider edge cases
We can break down each of these steps.
1. Understand the high-level purpose
Understanding the business context of a PR is important to determine the customer impact. A few questions you might ask yourself are:
What changes does this introduce to my product or service?
Depending on your team’s norms, the change introduced by the PR is likely outlined in the description section of the PR. If it is not, consider checking your project management tool or reaching out to the PR author.
How does this impact stakeholders?
The feature change informs what stakeholders will be impacted. Consider existing customers as well as future customers.
Are all stakeholders aware of the change being made?
Once you have understood the details of the change and who will be impacted you are in a position to verify if those customers are aware of the pending changes. Consider whether product documentation needs to be updated or release messages prepared.
2. Review the code
Once you understand the high-level purpose of the PR, you’re ready to jump into the code. Ensure that the code achieves its objectives, follows engineering best practices, and adheres to style guidelines. Some aspects you might assess:
Is there existing code that can be reused to accomplish the same result?
Less is more. This applies to coding as well. If functionality has already been implemented elsewhere in the code base, utilize this code to avoid duplication. Additionally, leveraging libraries can also be a great way to clean up code —no need to reinvent the wheel.
Are any parts of the code confusing?
Never be afraid to ask questions, especially if there are parts of the code that you do not understand. These moments can be great learning opportunities for you and the PR reviewer. If there is a cleaner way to implement the solution, share your idea and be open to hearing why the PR author may have chosen to implement it another way.
Does the code style make the code more readable?
Code should be as clear as possible. If variable or function names are unclear, suggest a more descriptive name. Assess if parts of the code could use additional comments, or if existing comments complement the code as intended.
3. Test any changes
Determine whether the PR works as expected by testing the changes. This testing process will vary depending on your product. You might consider:
Have automated tests passed with these changes?
Automated tests could include smoke tests to verify essential functionality, performance tests to evaluate responsiveness and stability, integration tests to ensure all components work together as expected, or acceptance tests to verify the product has the intended end-user experience. Verify the automated tests your team uses run and succeed with this code change.
Have the changes been tested manually?
Depending on team norms, the PR author and/or the reviewer will manually test PR changes. If the PR author is responsible for solely testing the changes, screenshots should be added to the PR description to show the tests that were run along with the successful completion of those tests. For API changes, you might use Postman for testing. For frontend changes, you might render the page using localhost. You might also deploy infrastructure to a dev environment and verify it behaves as expected. Determine what steps you need to take to ensure the changes have been tested.
Are the test results consistent with what the PR author intended?
Confirm the desired end behavior with the PR author if there is anything you are unsure about. Additionally, verify that manual tests are covered by the PR test suite.
4. Consider edge cases
Now that you understand the business context, have reviewed the code, and are familiar with the existing tests, you can brainstorm edge cases. Ask yourself:
What edge cases are not covered?
Consider both existing and future customers here. Think about unexpected situations that might arise with the product. Are all potential customer cases accounted for?
How might the PR author account for these cases in the PR test suite?
For any edge cases that you identify, test the edge case manually if possible and share the case with the PR author. Collaborate with the PR author to determine if the current end behavior is desired, and if the edge case can be added to the test suite.
Tailor the PR review process to suit your needs
I encourage you to reflect on the review process outlined above, and determine how you might apply it to your own PR reviews. Consider how it applies to your team and propose new team norms for the PR review process if you believe that it may benefit your team.
By following the 4-step process outlined in this article, you can elevate your code collaboration efforts, ensure code quality, and contribute to building a resilient code base. Understanding the high-level purpose, reviewing the code, testing changes, and considering edge cases are the cornerstones of effective pull request assessments. Happy coding and reviewing!