zydronium/manage.get.gov
1
0
Fork 0
mirror of https://github.com/cisagov/manage.get.gov.git synced 2025-07-04 10:13:30 +02:00
Code Issues Projects Releases Packages Wiki Activity

Incorporate feedback

Browse source
This commit is contained in:
Erin Song 2024-10-07 11:30:31 -07:00
parent c20f7e6679
commit ec74cfb24d
No known key found for this signature in database
2 changed files with 9 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

13
.github/pull_request_template.md vendored
View file

@ -1,6 +1,6 @@
## Ticket ## Ticket
Resolves #001 Resolves #00
## Changes ## Changes
@ -44,12 +44,14 @@ Resolves #001
#### Ensured code standards are met (Original Developer) #### Ensured code standards are met (Original Developer)
<!-- Write N/A if the below code standards are not applicable to your PR --> <!-- Write N/A if the below code standards are 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 - [ ] Interactions with external systems are wrapped in try/except
- [ ] Error handling exists for unusual or missing values - [ ] Error handling exists for unusual or missing values
#### Validated user-facing changes (if applicable) #### 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 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 - [ ] Checked keyboard navigability
- [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI) - [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
@ -58,10 +60,10 @@ Resolves #001
#### Reviewed, tested, and left feedback about the changes #### Reviewed, tested, and left feedback about the changes
- [ ] Pulled this branch locally and tested it - [ ] Pulled this branch locally and tested it
- [ ] Verified code meets above code standards and user-facing checks. Addresses any checks that are not satisfied - [ ] Verified code meets above code standards and user-facing checklist. Address any checks that are not satisfied
- [ ] Reviewed this code and left comments. Indicate if comments must be addressed before code is merged - [ ] 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 - [ ] Checked that all code is adequately covered by tests
- [ ] If any model was updated to modify/add/delete columns, verified migrations can be run with `makemigrations` - [ ] Verify migrations are valid and do not conflict with existing migrations
#### Validated user-facing changes as a developer #### 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 **Note:** Multiple code reviewers can share the checklists above, a second reviewers should not make a duplicate checklist
@ -92,6 +94,9 @@ Resolves #001
- [ ] Safari - [ ] Safari
- [ ] (Rarely needed) Tested as both an analyst and applicant user - [ ] (Rarely needed) Tested as both an analyst and applicant user
### References
- [Code review best practices](../docs/dev-practices/code_review.md)
## Screenshots ## Screenshots
<!-- If this PR makes visible interface changes, an image of the finished interface can help reviewers <!-- If this PR makes visible interface changes, an image of the finished interface can help reviewers

1
docs/dev-practices/code_review.md
View file

@ -7,7 +7,6 @@ After creating a pull request, pull request submitters should:
- Add at least 2 developers as PR reviewers (only 1 will need to approve). - Add at least 2 developers as PR reviewers (only 1 will need to approve).
- Message on Slack or in standup to notify the team that a PR is ready for review. - Message on Slack or in standup to notify the team that a PR is ready for review.
- If any model was updated to modify/add/delete columns, run makemigrations and commit the associated migrations file. - If any model was updated to modify/add/delete columns, run makemigrations and commit the associated migrations file.
- If any updated dependencies on Pipfile, also update dependencies in requirements.txt.
## Pull request approvals ## Pull request approvals
Code changes on user-facing features (excluding content updates) require approval from at least one developer and one designer. Code changes on user-facing features (excluding content updates) require approval from at least one developer and one designer.