Pull Request Process & Standards
Goalsโ
Ensure correctness, maintainability, and alignment with architecture, not perfectionism; small, focused PRs are strongly preferred. PRs should be readable and provide enough testing context for the reviewer to feel confident merging into the codebase.
Timelinesโ
Aim to respond to review requests within two working days. If a review has gone on longer than two working days, bring to the team's attention during daily standup. Occasionally there will be instances where a PR needs to be held until a specific time. The PR should be kept in draft if it does not need review, or it should be labeled 'DO NOT MERGE' if it can be reviewed but not merged. Typically, we would opt for a feature gate over holding back a branch from release.
Processโ
It is up to the individual reviewer on how exactly they handle moving through a review, but they should both review the underlying code and test the design and functionality.
Visual QAโ
Engineers should follow the instructions outlined in the testing section of the PR. Some PRs are better tested locally, while others are only testable in a deployed environment (such as the deployed oxygen preview). It is expected that the PR is tested in both mobile and desktop viewports. Verify edge cases as well as check other possible impacted areas of the site for any regression issues. If an issue is found, provide as much detail as possible to replicate it. This includes screenshots, Jam/Loom videos, device/browser info, steps to replicate, etc.
Code Reviewโ
When reviewing the code itself, the PR should be reviewed for best practices, scalability, and extendibility. We also want to ensure that the code follows existing patterns within the application, and if it breaks these, the reasoning is documented and understood. Reviewers should ask questions about approaches, spark conversation, and provide constructive feedback.
BirdyBotโ
As part of our CI process, Birdy Bot will review each PR based on a prompt given to it in the codebase. If Birdy Bot flags any critical issues, it can request changes on a PR, but it can never approve a PR. Reviewers should note feedback given by Birdy Bot, and if valid, ensure it is resolved before approving. While this tool is incredibly useful, it is important that reviewers still follow the process above to validate beyond Birdy Bot's review.
Review Standardsโ
- Comment on behavior and design more than style
- Review desktop and mobile viewports
- Use automated checks for linting and tests
- Provide constructive, easy to understand feedback (schedule discussion if necessary)