-
Notifications
You must be signed in to change notification settings - Fork 20
Pull request creation and review
This is mostly for active team members on the project but others can follow as well.
Be default the 'main' branch is protected so changes have to be made via pull requests. Active team members will be given access to the repos to create branches and submit pull requests.
Create a pull request by pushing up your branch with changes to the repo and then following GitHub pull request process. In the pull requests do the following:
- Set yourself as the assignee.
- Select reviewers. Most common is dmundra and mgifford.
- In the description:
- Enter the issue number you are working on (e.g. https://github.com/GSA/openacr/pull/266) and if the pull request can close or fix the issue say 'Closes ' or 'Fixes ' (e.g. https://github.com/GSA/openacr-editor/pull/26).
- Enter a note about what the changes are. You can use the commit messages for this.
- If QA instructions can be provided please provide step by step instructions. E.g. https://github.com/GSA/openacr-editor/pull/30
- Click create the pull request.
- Move the original issue to the 'Review' column or similarly applicable status.
If you are working on a draft pull request skip adding reviewers and moving the original issue to a review status till you are ready for review.
All of our repos have some kind of automation (called GitHub actions) that occurs on pull request creation. They are defined in the .github/workflows folder. They mostly consist of:
- Running tests. This is either mocha or cypress tests.
- Running accessibility test. Note: our current pa11y-ci tests sometime fail and need to be rerun.
- Running pre-commit checks and changes. This will do linting and submit a commit to the pull request if changes are to be made. E.g. https://github.com/GSA/openacr/pull/214 (look for pre-commit fixes commit).
- Running deployment. This is either to GitHub pages or federalist.
As a reviewer, please review the pull request and make sure to do the following:
- Confirm the description has details, issue number, QA instructions, and is clear for you to review the changes.
- If not clear, feel free to comment asking for more details from the assigner.
- You can also assign the someone else to review if you want a second look or want someone else to review.
- The GitHub actions has finished running and is all green.
- If it is red please review the error and rerun if you can.
- If still red, review the error and share it as a comment.
- If you can troubleshoot the cause of the error, please point out the possible location of the issue for assigner to fix.
- Click on the 'Files changed' tab and review the file changes.
- Add comments about any changes that you have a question about or feel like should be done a different way. Be curious and be open to share and receive feedback.
- Click 'Review changes' and if things are all good, add a comment and select 'Approve' then 'Submit review'.
- If you have changes you would like to see made, add a comment and select 'Request changes' then 'Submit review'.
Pull requests require at least one approver and the actions to pass to be mergeable. We merge using 'Squash and merge' to keep our commit history small.
Once the pull request is approved the assignee is in charge to merge the changes unless they are away or allow the team member to merge the changes.
Feel free to enable auto merging so that when someone approves it, it will be merged right away.
Note: Squashing and merging can cause conflicts if you have a lot of local branches so I recommend pull doing changes and rebasing local branches after every merge sooner rather than later.