How to Review Pull Requests
Reviewing pull requests (PRs) is a crucial part of maintaining code quality and fostering collaboration within an open-source project.
Here's a comprehensive guide on effectively reviewing pull requests in our repository:
Navigate to the Pull Requests tab on the GitHub repository page.
Click here to see the PR tab on Github
Make sure the filters `is:pr` and `is:open` are active.
Click on a Pull Request to review.
Click here to assign yourself.
Navigate to the linked issue that the Pull Request fixes to understand the requested changes.
Review the list of files changed to see how the issue was fixed.
Checkout the branch listed in the PR with git checkout <remote_branch_name> and run the app locally with npm run start to see the changes. Ask yourself these questions:
1. Does it solve the issue listed?
2. Does it adhere to Expunge Assist's existing coding conventions?
3. Should it be refactored?
If the issue linked to the PR has a frontend label, it's front-end focused and likely contains a design. As a result, most reviews will need to contain screenshots to communicate design needs specifically.
When reviewing a frontend issue, we want to consider a few things:
1. Responsiveness
2. Accessibility
3. Accuracy to Design
Even if Design doesn't provide figma design mockups for every breakpoint, we are responsible for making sure the website is responsive at all breakpoints.
Test for Responsiveness using manual testing using the browser devtools or tools like the chrome extension, Responsive Viewer.
If you find any oversights, take a screenshot or screen recording and add it to your PR review. Be specific about the breakpoints where the bug is present.
After a large accessibility audit, our site is accessible 🎉 and we'd like to keep it that way.
When building new components or pages from scratch, it's important to keep in mind how it will translate to screen readers and keyboard navigation.
Many frontend issues come down to implementing Design's vision to the website. This means that as developers we need to aim for accuracy when translating the Figma designs to the website.
Compare the review branch to the Figma and call out any differences between the two in your review.
Start a formal review on the pull request.
-
Comments:
- Leave a comment on the PR conversation, or add inline comments directly on the code for specific suggestions or questions.
- Use clear and constructive language to communicate feedback.
-
Overall Feedback:
- Summarize your overall impressions of the pull request.
- Highlight any significant concerns or areas for improvement.
-
Approval:
- If the pull request meets the project's standards and requirements, approve it.
- Provide a clear rationale for your approval.
- Assign it to the Dev Lead to be merged.
-
Changes Requested:
- If the PR needs updates, leave specific comments on the PR for the developer to address. Tag the developer and update status to
request changes.
- If the PR needs updates, leave specific comments on the PR for the developer to address. Tag the developer and update status to
-
Discussion:
- Engage in a constructive dialogue with the author to clarify any feedback or questions.
- Offer assistance and guidance to help resolve any issues identified during the review process.
Once you've approved the PR, you're all done 🎉
Providing thorough and constructive feedback helps maintain code quality and supports a collaborative development environment.
Happy reviewing!