Code review best practices: How to complete a pull request

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. 

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.

1. Understand the high-level purpose

2. Review the code

3. Test the changes

4. Consider edge cases

We can break down each of these steps.

Understanding the business context of a PR is important to determine the customer impact. A few questions you might ask yourself are:

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.

The feature change informs what stakeholders will be impacted. Consider existing customers as well as future customers.

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.

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:

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.

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.

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.

Determine whether the PR works as expected by testing the changes. This testing process will vary depending on your product. You might consider:

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.

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.

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.

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:

Consider both existing and future customers here. Think about unexpected situations that might arise with the product. Are all potential customer cases accounted for?

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.

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!