title | description | layout | category | subcategory |
---|---|---|---|---|
Pull Request Process |
How we do code reviews in pull requests |
article |
Development |
Code |
See the overall TTS Engineering Code Review guidelines for generally applicable advice about creating a positive environment for code reviews.
These conventions apply to [Login.gov 18f/identity-*
repositories in GitHub][github-repositories]
for contributions from the Login.gov team (anybody in the
[18f/identity-core
group][github-permissions]. See
external contributions for more information on
changes from outside.
[github-repositories]: {% link _articles/github-gitlab.md %}#repositories
-
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
][github-permissions] 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.
- In general, any single approving review from another [
-
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.
[github-permissions]: {% link _articles/github-gitlab.md %}#permissions
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.
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.
The handbook's GitHub article includes information about other GitHub features, such as:
- [Running continuous integration outside of a pull request]({% link _articles/github-gitlab.md %}#running-ci-outside-of-a-pull-request) or [sharing work with other engineers]({% link _articles/github-gitlab.md %}#sharing-work-with-other-engineers)