HOW-TO-REVIEW-A-DEVELOP-PR.md

How to Review a Develop Pull Request(PR)

tl;dr commands: (unless it's a forked repo, see commands here)

git fetch upstream
git checkout 45-add-waffle-badges //for example
yarn
yarn test
// Press a to run all tests
// Press q to quit watch mode.
yarn start
// Navigate to http://localhost:3000, then review and inspect the app and its elements for expected and unexpected behavior

Then skip to the Reviewing from GitHub section

Sing for Needs procedures for merging Pull Requests dictates one review by a fellow collaborator.

• This way, before the merge takes place, we have some quality control, and allow the collaborator making the request to improve their code.

• So you're ready to contribute by reviewing a pull request? Great! We'll walk you through the process step by step.

• We're going to assume the PR is for 45-add-waffle-badges.

• First, go to your terminal and fetch any new branches with git fetch upstream.

• Then, navigate to the branch which you want to provide a PR review with git checkout 45-add-waffle-badges.

• Once on the PR branch, fetch and merge with git pull.

• Now, you are ready to run the tests to verify everything is passing. For this, you'll need to run yarn && yarn test.

• Having verified all the tests pass, you can start the server with yarn start and navigate to http://0.0.0.0:3000 in your browser to check that everything compiles correctly.

Reviewing from GitHub

• The next step is to navigate to SFN-Client's repo.

• Click on the Pull Request tab to see all available PR's.

• Find the Pull Request you want to review and click on it.

• Once on the PR, in this case, 45-add-waffle-badges, at the bottom of the page you will see Review required.

• Before adding your review, check to make sure the branch is not out of date with the base branch, and if it is, Update branch.

• If the branch is up-to-date, it will look like this:

with Merge pull request blocked because it needs to be reviewed.

• Click on the Files Changed Tab to review all file changes.

• You can click on Diff Settings dropdown to choose Unified form, rather than the default Split to see the changes more clearly.

• You can also add comments to a particular line of code, by clicking on the plus icon when you hover over a line.

• You can then either just provide the single comment or start a review, if you haven't, yet.

• Clicking on Add your review will take you to the page where you can see the changes and Comment, Approve, or Request changes, as appropriate.

• Assuming you approve the Pull Request, you will be taken back to the PR and prompted to Merge pull request with everything in green.

• Then, Confirm merge to finish your PR review.

Troubleshooting

• Sometimes, there will be conflicts that must be resolved before making your review.

• Working through conflicts, via the command line, is straightforward using these instructions, provided by GitHub.

• If you prefer, you can also click on the Resolve conflicts link which will take you to the merge conflicts.
• For each file with a merge conflict, you'll click on Mark as resolved so you can move on to the next one:

• View the following file, and click Next to cycle through the conflicts in the file, if there are more than one:

• Be sure to remove the branch name comments and the equals sign line, as you decide how to resolve:

• Click on Commit merge to kickoff the full update of the branch:

• As the develop branch merges, you will be taken back to the Pull Request where you can make your review.

  

• Check out This Link for tips on reviewing React code.

• Thank you for your collaboration in reviewing Pull requests. This is an essential part of working as a team to improve code.

For forked repos

tl;dr commands (actually, if you read the step-by-step instructions for a non-forked repo, then these are very similar.

With this example, you can run the following commands:

git remote add mike https://github.com/aonomike/sfn-client
git fetch mike
git checkout 98_refactor-tests-to-use-enzyme
git pull
yarn && yarn test
// Press a to run all tests
// Press q to quit watch mode.
yarn start
// Navigate to http://localhost:3000, then review and inspect the app and its elements for expected and unexpected behavior

Then jump back to the Reviewing from GitHub section

What happens after?

• The reviewer should notify on Slack the owner of the PR if some comments made on the PR, and additional work is expected by the owner of the PR within the current scope

• If the reviewer found work outside of the current ticket's scope, the reviewer should open a new issue (and optionally notify on Slack the owner of the current PR)

• If the PR gets outdated, then it's the reviewer's responsibility to auto-update the branch to the current state. If the auto-update fails, the reviewer should notify the owner of the PR to update the PR