From a41c3fe66404a398da7a790d4e5a35d0bf2cf8dd Mon Sep 17 00:00:00 2001 From: "Daniel D. Beck" Date: Tue, 24 Sep 2024 17:00:16 +0200 Subject: [PATCH 1/8] Contributor docs: write up merge practices While the governance doc covers some _rules_, it doesn't catch some of our unwritten conventions. I've written this up in anticipation of adding new peers. --- docs/reviewing.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 docs/reviewing.md diff --git a/docs/reviewing.md b/docs/reviewing.md new file mode 100644 index 00000000000..31a20f18b3d --- /dev/null +++ b/docs/reviewing.md @@ -0,0 +1,32 @@ +# Pull request and issue management + +## Merging pull requests + +> [!NOTE] +> This information is for [project peers and owners](../GOVERNANCE.md#roles-and-responsibilities). + +If a pull request has been approved and tests are passing, then you can merge it. +Follow these steps to merge: + +1. Make sure **you** can merge this pull request. + Some changes—most notably, feature removals, tooling and policy changes, and schema changes—require an owner to merge. + See [the privileges and responsibilities matrix](../GOVERNANCE.md#privileges-and-responsibilities-matrix) for more information. + +1. Make sure there are no explicit blockers to merging. + Blockers include: + + - The pull request has the [blocked label](https://github.com/web-platform-dx/web-features/pulls?q=is%3Aopen+is%3Apr+label%3Ablocked) + - A reviewer asked to re-review before merging or has requested changes that have not been addressed with a commit or comment + - The author asked to delay merging + + If a blocker no longer applies, remove the label or dismiss the stale review. + +1. If the pull request might require a Semantic Versioning MAJOR or MINOR release, then add the [minor version required](https://github.com/web-platform-dx/web-features/labels/minor%20version%20required) or [major version required](https://github.com/web-platform-dx/web-features/labels/major%20version%20required) label. + +1. Click the **Squash and merge** button, then prepare the commit message. + + Clean up the subject and message fields so they make sense as a single unit. + For example, delete boilerplate messages about applying changes from code review. + Often an imperative-noun subject, such as "Add example feature," is sufficient. + +1. When you're ready to commit the changes, click **Confirm squash and merge**. From 80f65f3ea77c34a516808de7f5912bdaafdc09de Mon Sep 17 00:00:00 2001 From: "Daniel D. Beck" Date: Tue, 24 Sep 2024 17:46:19 +0200 Subject: [PATCH 2/8] Elaborate on how review requests ought to be interpreted --- docs/reviewing.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/reviewing.md b/docs/reviewing.md index 31a20f18b3d..e7ce45c2a31 100644 --- a/docs/reviewing.md +++ b/docs/reviewing.md @@ -17,7 +17,8 @@ Follow these steps to merge: - The pull request has the [blocked label](https://github.com/web-platform-dx/web-features/pulls?q=is%3Aopen+is%3Apr+label%3Ablocked) - A reviewer asked to re-review before merging or has requested changes that have not been addressed with a commit or comment - - The author asked to delay merging + - The author explicitly asked to delay merging + - The author explicitly asked fro review from all requested reviewers (the presumption is at least one but not all) If a blocker no longer applies, remove the label or dismiss the stale review. From 4f54103e0d6e76aed9ebee0130f7bca4fcb6c4f9 Mon Sep 17 00:00:00 2001 From: "Daniel D. Beck" Date: Wed, 25 Sep 2024 12:26:52 +0200 Subject: [PATCH 3/8] Apply suggestions from code review Co-authored-by: James Stuckey Weber --- docs/reviewing.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reviewing.md b/docs/reviewing.md index e7ce45c2a31..e0fe66900ae 100644 --- a/docs/reviewing.md +++ b/docs/reviewing.md @@ -18,7 +18,7 @@ Follow these steps to merge: - The pull request has the [blocked label](https://github.com/web-platform-dx/web-features/pulls?q=is%3Aopen+is%3Apr+label%3Ablocked) - A reviewer asked to re-review before merging or has requested changes that have not been addressed with a commit or comment - The author explicitly asked to delay merging - - The author explicitly asked fro review from all requested reviewers (the presumption is at least one but not all) + - The author explicitly asked for review from all requested reviewers (the presumption is at least one but not all) If a blocker no longer applies, remove the label or dismiss the stale review. @@ -28,6 +28,6 @@ Follow these steps to merge: Clean up the subject and message fields so they make sense as a single unit. For example, delete boilerplate messages about applying changes from code review. - Often an imperative-noun subject, such as "Add example feature," is sufficient. + Often an imperative-noun subject, such as "Add example feature", is sufficient. 1. When you're ready to commit the changes, click **Confirm squash and merge**. From fefab53ffdda951de9b05f25a86ba0e2c17930da Mon Sep 17 00:00:00 2001 From: "Daniel D. Beck" Date: Wed, 25 Sep 2024 12:28:45 +0200 Subject: [PATCH 4/8] Add step suggested by @jamesnw Co-authored-by: James Stuckey Weber --- docs/reviewing.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/reviewing.md b/docs/reviewing.md index e0fe66900ae..d767e69e8e3 100644 --- a/docs/reviewing.md +++ b/docs/reviewing.md @@ -8,7 +8,10 @@ If a pull request has been approved and tests are passing, then you can merge it. Follow these steps to merge: -1. Make sure **you** can merge this pull request. +1. Make sure you **should** merge this pull request. + In general, only merge pull requests you are already familiar with, either as an author or reviewer. + +1. Make sure **you** should merge this pull request. Some changes—most notably, feature removals, tooling and policy changes, and schema changes—require an owner to merge. See [the privileges and responsibilities matrix](../GOVERNANCE.md#privileges-and-responsibilities-matrix) for more information. From 3f3f86269b492427500a3c0f0ea1d9a29f6e83ef Mon Sep 17 00:00:00 2001 From: "Daniel D. Beck" Date: Wed, 25 Sep 2024 17:46:19 +0200 Subject: [PATCH 5/8] Add periods to complete sentences Co-authored-by: Patrick Brosset --- docs/reviewing.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/reviewing.md b/docs/reviewing.md index d767e69e8e3..aac10b46541 100644 --- a/docs/reviewing.md +++ b/docs/reviewing.md @@ -18,10 +18,10 @@ Follow these steps to merge: 1. Make sure there are no explicit blockers to merging. Blockers include: - - The pull request has the [blocked label](https://github.com/web-platform-dx/web-features/pulls?q=is%3Aopen+is%3Apr+label%3Ablocked) - - A reviewer asked to re-review before merging or has requested changes that have not been addressed with a commit or comment - - The author explicitly asked to delay merging - - The author explicitly asked for review from all requested reviewers (the presumption is at least one but not all) + - The pull request has the [blocked label](https://github.com/web-platform-dx/web-features/pulls?q=is%3Aopen+is%3Apr+label%3Ablocked). + - A reviewer asked to re-review before merging or has requested changes that have not been addressed with a commit or comment. + - The author explicitly asked to delay merging. + - The author explicitly asked for review from all requested reviewers (the presumption is at least one but not all). If a blocker no longer applies, remove the label or dismiss the stale review. From 14ac897d1cff4002bcea8384876cf8cf6e6de28d Mon Sep 17 00:00:00 2001 From: "Daniel D. Beck" Date: Wed, 25 Sep 2024 17:49:11 +0200 Subject: [PATCH 6/8] Link to GitHub dismiss stale review docs --- docs/reviewing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reviewing.md b/docs/reviewing.md index aac10b46541..3fe8e255bcd 100644 --- a/docs/reviewing.md +++ b/docs/reviewing.md @@ -23,7 +23,7 @@ Follow these steps to merge: - The author explicitly asked to delay merging. - The author explicitly asked for review from all requested reviewers (the presumption is at least one but not all). - If a blocker no longer applies, remove the label or dismiss the stale review. + If a blocker no longer applies, remove the label or [dismiss the stale review](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/dismissing-a-pull-request-review). 1. If the pull request might require a Semantic Versioning MAJOR or MINOR release, then add the [minor version required](https://github.com/web-platform-dx/web-features/labels/minor%20version%20required) or [major version required](https://github.com/web-platform-dx/web-features/labels/major%20version%20required) label. From eab4c22488f5958606a642b275dc78e4d54f71da Mon Sep 17 00:00:00 2001 From: "Daniel D. Beck" Date: Wed, 25 Sep 2024 17:53:04 +0200 Subject: [PATCH 7/8] Cross-reference reviewing in `CONTRIBUTING.md` --- docs/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 4f238a922c7..1c7091477e8 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -16,7 +16,7 @@ There are multiple ways to contribute to the web-features repository, such as: * Adding a missing feature to the repository, from scratch. * Adding a missing feature to the repository, by updating a feature that was already drafted. * Updating an existing feature, for example to change its name, description, support data, or other fields. -* Reviewing a pull request that was submitted by someone else, to check if the feature is correctly authored. +* [Reviewing a pull request](./reviewing.md) that was submitted by someone else, to check if the feature is correctly authored. In any case, **thank you** for wanting to help. This document will guide you through the process of contributing to the web-features repository. From 38d96d19145bf897346e51cd63caa823ce2faad4 Mon Sep 17 00:00:00 2001 From: "Daniel D. Beck" Date: Wed, 25 Sep 2024 17:55:36 +0200 Subject: [PATCH 8/8] Cross-reference reviewing in `./README.md` --- docs/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/README.md b/docs/README.md index 53f1d9b3029..cc99dd7733f 100644 --- a/docs/README.md +++ b/docs/README.md @@ -2,4 +2,5 @@ * To learn to contribute to the web-features project, see [Contributing to the web-features project](./CONTRIBUTING.md). * For guidelines on writing features, see [Feature guidelines](./guidelines.md). +* For guidelines on reviewing and merging changes, see [Pull request and issue management](./reviewing.md). * For information about what Baseline is and how Baseline status is calculated, see [Baseline](./baseline.md).