Pull Request Process

General Code Review Guidelines

See the overall TTS Engineering Code Review guidelines for generally applicable advice about creating a positive environment for code reviews.

Reviewing and Merging

These conventions apply to Login.gov 18f/identity-* repositories in GitHub for contributions from the Login.gov team (anybody in the 18f/identity-core group. See external contributions for more information on changes from outside.

  • Who reviews: at least one member of the engineering team must review and approve a pull request before it can be merged.

    • In general, any single approving review from another 18f/identity-core member is adequate. However, in some repositories with default reviewer groups (such as identity-dashboard or identity-dev-docs with clear owners) it is preferred to wait for a reviewer from those groups before merging.

  • Who merges: the author merges their own pull request.

    • In rare circumstances where a merge is time-sensitive and the pull request author is unavailable, it may be acceptable for another to merge on their behalf.

External Contributions

External contributions are welcome! For security reasons, our continuous integration pipeline will not run on branches from outside contributors. Since external contributors don’t have write access, a Login.gov team member will need to run the tests themselves and merge the code.

⛔️ “Changes Requested”

GitHub lets reviewers Approve, Comment, or Request Changes when submitting a review. The Request Changes option will block merges until that specific reviewer approves the Pull Request, or until that review is dismissed.

Since Request Changes is essentially blocking, as a Login.gov convention, we try to minimize usages of it to absolutely critical changes needed.

Using GitHub Functionality Without Submitting a Pull Request

The handbook’s GitHub article includes information about other GitHub features, such as: