How to prepare and review pull requests - DOI-ONRR/nrrd GitHub Wiki

Pull requests (PRs) happen when we want to merge a working branch into another branch. All of our pull requests merge new features or fixes into dev, then dev is merged into master (the live site).

We use PRs to review, validate, and discuss work. We sometimes use work-in-progress PRs, denoted by WIP, to share and collaborate on ongoing work before we're ready to merge.

For PRs that are potentially ready for merging, there are some steps we generally follow to make sure all goes well.

Preparing a Pull Request (PR) for Review

Set the PR up to go from your current branch into dev. Ensure that you have selected the branch that you intend to merge under the “compare” dropdown. The dev branch is the base default, so you don’t need to change that.
Following the PR template, fill in the PR with the GitHub issue(s) it addresses, a preview link, and a description of the changes.
Assign at least one other team member to review that PR. You should assign the team member(s) with the most relevant knowledge and skill set:
- Code related changes should have at least either the developer or content strategist review them.
- Content related changes should have at least the content strategist, project manager, or program analysts review them.
- PRs related to a new design should have the user experience designer review them.
- Visual design related PRs should have at least the content strategist or user experience designer review them.

Reviewing a PR

Basic steps

Did CircleCI tests pass? Did the site build? (These can take 2-10 minutes, so don't panic if they're not immediate.) If either failed, proceed with extra caution and pull in a technical reviewer.
Click on the preview link. (If there’s no preview link, use the format https://https://nrrd-preview.app.cloud.gov/sites/BRANCH-NAME/ to find the preview.) Does everything look as you expect? If you expect widespread changes (or are worried there may be unintentionally widespread changes), here's a list of what to check.
Navigate to any pages in the preview you expect to be different because of the changes, and verify that the changes look right. Also check any pages that might be incidentally affected, and make sure they look as you expect (especially for style or template changes).
Look at the "Files changed" tab in the pull request. Are these the files you expect to have changed? Do the changes make sense and seem intentional? If there is an unexpectedly long list of files changed (for instance, 259 files changed for a typo fix), pull in a technical reviewer and check with the person who opened the PR. If the change includes a data update, make sure the appropriate yml files have changed.

Additional checks for refactoring, data changes, or anything that might have wider-reaching effects

If any data has changed, check both national and state-level pages. If templates have changed, check several pages that rely on the templates. For instance, if data or explore-data templates have changed, check states with very different data (i.e. KS, ME, TX, and AK) to make sure they all look right.
Sometimes, I find it helpful to open up both the current site (either the live version or on dev), and compare the preview side-by-side. This is especially useful for style changes, to ensure the styles cascade appropriately.

After reviewing

If you're comfortable the changes are ready to merge:

click "Add your review" and approve the changes
or comment to say it looks good

After the PR has been reviewed and approved, the product manager or the team member acting in that capacity will review and merge the PRs to the 'dev' branch.

After the PR is merged, delete the temporary/working branch (unless it’s dev, which doesn’t get deleted)

If you see something that needs work, or you think it needs additional review, you might:

use "Add your review" to request changes
add a reviewer
comment with a note about what should change
comment to tag an additional reviewer