Pull request process

Not all PRs have associated issues, so some steps may not always be relevant and may be skipped. It's easier to add this note once at the beginning of this document than to repeat caveats throughout.

Creating a pull request

1. In Jira, move the issue from "To Do" to "In Progress". Jira will automatically assign the issue to you. Make sure the issue is fully defined at this point.
2. Create a feature branch or bug fix branch, off of the master branch, ideally with the issue number (e.g. git checkout -b fix/SF-123-password-page-error).
3. Commit relevant changes with a commit message like SF-123 Fix error on change password page.
4. Push the branch to GitHub with git push -u origin fix/SF-123-password-page-error. This assumes the remote is named origin (run git remote to list remotes). The -u will set the branch to track the remote branch you are creating when you push.
5. Open a pull request in GitHub. Explain any relevant context for the issue that may not be obvious to the reviewer of your pull request. Apply the label "will require testing" unless the change is such that having the test team approve it first is not needed (e.g. no changes to production code), too much of a hindrance (e.g. for a hotfix), or extremely unlikely to be a meaningful test (e.g. change fixes a problem that only developers are likely to reproduce, and isn't likely to break anything).
6. Move the issue in Jira from "In Progress" to "Code Review". If possible, add acceptance criteria to the issue.
7. Check back later to see and address any feedback.
8. If changes are needed, make the changes and push them to the branch (force pushing is perfectly acceptable). Consider rebasing your branch on master at this point (in general this is a good idea because it moves it closer to what it will be once merged, so any testing is more likely to yield useful results).
9. Notify the reviewer of what changes you made.
10. If the pull request is approved, make sure the issue has been moved to "Ready to Test" in Jira, and that the issue has acceptance criteria. Also add the "ready to test" label to the pull request, if applicable.
11. Monitor the issue in Jira and address any feedback from the test team. This may result in code changes that require moving the pull request and the issue further back in the process.
12. Once the issue has passed testing, add the "testing complete" label in GitHub.
13. Once the pull request is merged (whether you do it or not), make sure the Jira issue gets moved from "Testing Complete" to "Helps" or "Done". If it impacts the helps, make sure to add a comment explaining what has changed and what the impact may be.

Reviewing a pull request

1. Assign yourself to the pull request so it will be clear to others that someone is taking on the task of reviewing it. This assignment should be seen as tentative and you can remove yourself if the situation changes. If someone is on vacation or otherwise away from the project, it can serve as an indicator of which pull requests are likely to be abandoned while that person is gone.
2. Review changes, and test the functionality, if possible. We usually use Reviewable, which has some great advantages, but only works in the browser. The GitHub Pull Requests and Issues extension can allow reviewing code directly in VS Code, though you lose the advantages of Reviewable. Both are acceptable options. One of Reviewable's strengths is keeping track of what changed since you last reviewed a pull request, so one option may be to review all the changes in VS Code, leave feedback, and then open Reviewable and mark all files as reviewed. When changes are made you can then review them in Reviewable, which will show you only what has changed. This strategy has not been tested yet.
3. If you request changes, check back later to see if there are changes to review.
4. Once you have approved the pull request, if there are no outstanding concerns from other reviewers, move the Jira issue from "Code Review" to "Ready to test". If the issue was not created by the test team, and it lacks acceptance criteria, either add acceptance criteria to the issue, or add a note to the issue asking the developer to add acceptance tests (e.g. if you think you lack the understanding of the issue to write them with a high degree of confidence). Once the issue is moved to "Ready to Test", remove the "will require testing" label from the pull request, and add the "ready to test" label.

Merging pull requests

1. Make sure there are no outstanding concerns raised on the pull request (some may be comments that aren't left as a "requesting changes" review).
2. Make sure testing is complete. Just because the issue is in the "Test Complete" column in Jira does not mean the test is fully passed. Check the issue for any comments.
3. If necessary, rebase the pull request so the CI can run with the PR based on master.
4. In general, merge the pull request by doing a squash and merge. Edit the commit message to make sure only meaningful information is there. The general format for commit messages is "SF-1234 Fix some bug (#123)", where the last number is the pull request number. Any non-helpful information in the commit message should be removed (e.g. messages about fixing up the PR before merging). In general, if it's not clear what something in a commit message means at a glance, it is not worth being part of the message and should be removed. If more information is needed the pull request number can be used to look up more information about the commit.

Exceptions

This document is intended to explain what the normal process should be, which does not always have to be followed. Here are some examples of when we may deviate from the usual process:

1. Hotfixes to live are generally not going to be tested by the test team
2. In some situations when an issue needs to be expedited, and there is high confidence that the code will be accepted without changes, it may make sense to move an issue to "Ready to Test" in Jira before a final code review is complete.