Skip to content

Commit

Permalink
Update pull request template to manage.get.gov template
Browse files Browse the repository at this point in the history
  • Loading branch information
erinysong committed Oct 17, 2024
1 parent 479d2f3 commit 535aa09
Showing 1 changed file with 16 additions and 38 deletions.
54 changes: 16 additions & 38 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
## Ticket

<!-- PR title format: `#issue_number: Descriptive name ideally matching ticket name - [sandbox]`-->
Resolves #00
<!--Reminder, when a code change is made that is user facing, beyond content updates, then the following are required:
- a developer approves the PR
- a designer approves the PR or checks off all relevant items in this checklist.
All other changes require just a single approving review.-->

## Changes

Expand Down Expand Up @@ -45,82 +41,64 @@ All other changes require just a single approving review.-->

- [ ] Met the acceptance criteria, or will meet them in a subsequent PR
- [ ] Created/modified automated tests
- [ ] Added at least 2 developers as PR reviewers (only 1 will need to approve)
- [ ] Messaged on Slack or in standup to notify the team that a PR is ready for review
- [ ] Changes to “how we do things” are documented in READMEs and or onboarding guide
- [ ] If any model was updated to modify/add/delete columns, makemigrations was ran and the assoicated migrations file has been commited.
- [ ] Update documentation in READMEs and/or onboarding guide

#### Ensured code standards are met (Original Developer)

- [ ] All new functions and methods are commented using plain language
- [ ] Did dependency updates in Pipfile also get changed in requirements.txt?
<!-- Mark "- N/A" and check at the end of each check that is not applicable to your PR -->
- [ ] If any updated dependencies on Pipfile, also update dependencies in requirements.txt.
- [ ] Interactions with external systems are wrapped in try/except
- [ ] Error handling exists for unusual or missing values

#### Validated user-facing changes (if applicable)

- [ ] New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
- [ ] Tag @dotgov-designers in this PR's Reviewers for design review. If code is not user-facing, delete design reviewer checklist
- [ ] Verify new pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
- [ ] Checked keyboard navigability
- [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
- [ ] Add at least 1 designer as PR reviewer

### As a code reviewer, I have

#### Reviewed, tested, and left feedback about the changes

- [ ] Pulled this branch locally and tested it
- [ ] Reviewed this code and left comments
- [ ] Verified code meets all checks above. Address any checks that are not satisfied
- [ ] Reviewed this code and left comments. Indicate if comments must be addressed before code is merged
- [ ] Checked that all code is adequately covered by tests
- [ ] Made it clear which comments need to be addressed before this work is merged
- [ ] If any model was updated to modify/add/delete columns, makemigrations was ran and the assoicated migrations file has been commited.

#### Ensured code standards are met (Code reviewer)

- [ ] All new functions and methods are commented using plain language
- [ ] Interactions with external systems are wrapped in try/except
- [ ] Error handling exists for unusual or missing values
- [ ] (Rarely needed) Did dependency updates in Pipfile also get changed in requirements.txt?
- [ ] Verify migrations are valid and do not conflict with existing migrations

#### Validated user-facing changes as a developer
**Note:** Multiple code reviewers can share the checklists above, a second reviewers should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.

- [ ] New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
- [ ] Checked keyboard navigability
- [ ] Meets all designs and user flows provided by design/product
- [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)

- [ ] Tested with multiple browsers, the suggestion is to use ones that the developer didn't (check off which ones were used)
- [ ] Chrome
- [ ] Microsoft Edge
- [ ] FireFox
- [ ] Safari

- [ ] (Rarely needed) Tested as both an analyst and applicant user

**Note:** Multiple code reviewers can share the checklists above, a second reviewers should not make a duplicate checklist

### As a designer reviewer, I have

#### Verified that the changes match the design intention

- [ ] Checked that the design translated visually
- [ ] Checked behavior
- [ ] Checked behavior. Comment any found issues or broken flows.
- [ ] Checked keyboard navigability
- [ ] Checked different states (empty, one, some, error)
- [ ] Checked for landmarks, page heading structure, and links
- [ ] Tried to break the intended flow

#### Validated user-facing changes as a designer

- [ ] Checked keyboard navigability
- [ ] Checked different states (empty, one, some, error)
- [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)

- [ ] Tested with multiple browsers (check off which ones were used)
- [ ] Chrome
- [ ] Microsoft Edge
- [ ] FireFox
- [ ] Safari

- [ ] (Rarely needed) Tested as both an analyst and applicant user

### References
- [Code review best practices](../docs/dev-practices/code_review.md)

## Screenshots

<!-- If this PR makes visible interface changes, an image of the finished interface can help reviewers
Expand Down

0 comments on commit 535aa09

Please sign in to comment.