-
Notifications
You must be signed in to change notification settings - Fork 3
Review Process
Emma Hogan edited this page Aug 9, 2022
·
16 revisions
- The reviewer should refer to the Review checklist when reviewing
- Once the developer receives notification that a review has been submitted, the developer should respond to each review comment:
- if changes are made in response to a review comment, reply to the comment with a link to the commit containing the change
- Changes can be committed to the branch and pushed at any time after the PR has been created; the PR will update automatically
- The shortened SHA renders as a link automatically in comments, see GitHub: Commit SHAs
- if no changes are made, reply to the comment to explain the reasons behind the decision not to make a change
- the developer should not click the
Resolve conversationbutton; only the reviewer should click this button when they approve the changes related to that review comment
- if changes are made in response to a review comment, reply to the comment with a link to the commit containing the change
- Once the developer has responded to each review comment, the developer should re-request a review, see GitHub: Re-requesting a review
- The reviewer should either:
- approve the change related to the review comment by clicking the
Resolve conversationbutton, see GitHub: Resolving conversations - submit additional review comments
- approve the change related to the review comment by clicking the
- If the developer and the reviewer agree that responding to a review comment would create an excessively large or unrelated change (taking into consideration that smaller changes may not be worked on if the change is postponed), open a new software task to make the change
- The above steps should be repeated until the reviewer has approved all changes
- Once the reviewer has approved all changes, the reviewer should approve the PR, see GitHub: Approving a pull request with required reviews
- Once the developer receives notification that the PR has been approved, the developer should Merge the pull request
- General:
- Do the changes:
- address the issue linked to from the PR?
- require changes to the user guide?
- require an entry to be added to
.gitignore?
- Have all checks on the PR passed?
- Do the changes:
- Code:
- Does the code follow the recommendations in the Developer Guide?
- Does the code validate? (follow Validate the code locally)
- Documentation:
- Does the documentation build? (follow Build the documentation locally)
- Do the HTML pages render correctly? Do all links work?
- Does the documentation correctly describe the code?
- Do the steps in the "Quick Start" section work?