manage.get.gov/.github/pull_request_template.md
2024-01-30 17:38:14 -05:00

5.1 KiB

Ticket

Resolves #00

Changes

  • Change 1
  • Change 2

Context for reviewers

Setup

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

  • 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 associated migrations file has been commited.

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?
  • 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
  • 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
  • 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 associated 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?

Validated user-facing changes as a developer

  • 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 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

  • 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

Screenshots