How To Review A Pull Request
By JoeVu, at: 15:00 Ngày 24 tháng 1 năm 2023
Receiving and reviewing pull requests is an essential part of the software development process. A pull request is a request to merge code changes from one branch into another branch, often from a developer's fork of a repository into a main repository. Pull requests allow developers to propose changes to a project, which can then be reviewed, discussed and accepted or rejected.
In this article, we’ll explain the process of reviewing a pull request and the steps that should be taken to ensure that the review is comprehensive and helpful.
Understand the issue
It’s important to understand what a pull request is and why it’s important. A pull request is a request from a developer to merge code changes from one branch into another branch. This is important because it allows developers to contribute to a project, while ensuring that the changes are reviewed before they are merged into the main branch.
Questions to be asked:
- What is the original issue/feature? How does the new feature work?
- How to replicate the issue?
- What if the change is not introduced?
Know the change
When reviewing a pull request, the reviewer should first look at the changs that were made. This includes looking at the code changes, as well as any documentation that was added or modified. The reviewer should also look at the commit messages and comments to ensure that the changes are appropriate and well-explained.
Questions to be asked:
- Does this change solve the issue completely or partly?
- Are all commit messages meaningful?
- Are code well tested? Is there any unit test? Does the test cover all cases?
The change impact
The reviewer should then look at the pull request’s effect on the project. This includes looking at any tests that were added or modified, as well as any other changes that may affect the project. It’s also important to look at the performance of the code changes, as this can have an impact on the project’s stability and scalability.
Questions to be asked:
- What is the side effect of the change on other features/code?
- What if the changed is rollbacked?
- Does this change migrate existing schemas and database?
- Does it affect other tests?
Make comments
Once the reviewer is satisfied that the changes are appropriate, they should leave a comment on the pull request, either accepting or rejecting the changes. If there are any concerns or questions, the reviewer should also leave a comment on the pull request to discuss the changes.
It’s important for the reviewer to provide constructive feedback on the changes. This can include pointing out any potential issues, as well as suggesting improvements that can be made. This feedback is essential for helping the developer to improve their code and for the project overall.
Questions to be asked:
- Is this code good or bad? Does it follow any coding style? Why?
- How to improve it?
- What are the other solutions?
- Is there any piece of code that the reviewer does not understand?
There are some rules/steps that we should follow:
- Understand the "Clean Code" philosophy
This is the fundamental things all coders must know in order to deliver a good code/product, this book provides helpful guidelines to write clean code. - Understand the best practices
Every single framework has its own best practices recommended by the community and experience developers, ex: Django Two Scoops is the best book for Django best practices - Follow the framework, and the team coding style
Having the same coding style for the entire team gain a lot of profit in terms of reviewing process, developing and debugging process. Everyone knows and easily grab others code and ideas. While, all members should follow the framework coding style so everything will be consistent. - Make sure the PR owner writing unit tests for his changes
Test is the secure and accuracy assurancy for the change, without test the new update is buggy. - Make sure the code is well-optimized
This is a must to check any particular pull request, having those questions below gives the reviewer better insight:- Is this memory usage wise, performance wise?
- Is there any better solution?
- Is there any redundant requests/queries?
- Provide constructive feedbacks
- Start with a positive. Begin your feedback by highlighting a positive aspect of the individual's work. This will help soften the blow of any criticism that follows.
- Explain what went wrong. Be specific when discussing any issues with the individual's work.
- Provide solutions. Offer suggestions on how to improve the situation and provide the individual with resources or information to help them make progress.
- Be open-minded. Listen to the individual's response and be willing to consider other perspectives.
- End on a positive note. Acknowledge progress made and reinforce the positive aspects of the individual's work.
Providing feedbacks is nevery easy, people are
- Reviewing means learning, take it seriously
Pay close attention when reviewing a pull request and to take the process of learning seriously. It is often used to remind developers to be diligent and conscientious when they are studying or reviewing others' changes they have already learned.
In summary, reviewing a pull request is an important part of the software development process. It’s important for the reviewer to look at the changes, evaluate their impact on the project, and provide constructive feedback. By following these steps, the reviewer can ensure that the pull request is comprehensive and helpful.