From 7e62efa1cc19d26e747a9cb1a6098225421a8468 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 11 Mar 2024 14:20:21 -0400 Subject: [PATCH 1/2] Clarify pull request approval by engineering --- _articles/pull-request-review.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/_articles/pull-request-review.md b/_articles/pull-request-review.md index fd50cadb..24b98ed2 100644 --- a/_articles/pull-request-review.md +++ b/_articles/pull-request-review.md @@ -21,8 +21,8 @@ changes from outside. [github-repositories]: {% link _articles/github.md %}#repositories -- **Who merges**: once there is at least one approving review, the author - merges their own pull request. +- **Who merges**: once there is at least one approving review by a member of + the engineering team, 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 From 8435cb56882828cfec46221d0538d24dad9fd346 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 11 Mar 2024 15:36:23 -0400 Subject: [PATCH 2/2] Refine "Who reviews" language remove redundancy, improve clarity by using specific language See: https://github.com/GSA-TTS/identity-handbook/pull/524/files#r1520249835 Co-authored-by: Zach Margolis --- _articles/pull-request-review.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/_articles/pull-request-review.md b/_articles/pull-request-review.md index 24b98ed2..bcebd882 100644 --- a/_articles/pull-request-review.md +++ b/_articles/pull-request-review.md @@ -21,22 +21,22 @@ changes from outside. [github-repositories]: {% link _articles/github.md %}#repositories -- **Who merges**: once there is at least one approving review by a member of - the engineering team, the author merges their own pull request. +- **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](https://github.com/18f/identity-dashboard) + or [identity-dev-docs](https://github.com/GSA-TTS/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. -- **Who reviews**: 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](https://github.com/18f/identity-dashboard) or - [identity-dev-docs](https://github.com/GSA-TTS/identity-dev-docs) with clear - owners) it is preferred to wait for a reviewer from those groups before - merging. - [github-permissions]: {% link _articles/github.md %}#permissions ## External Contributions