From a24d11fb9c0258f70ffda319040e256347b81b02 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 10 Aug 2018 14:49:21 -0700 Subject: [PATCH 1/6] doc: require two approvals to land changes Currently, changes require approval by one Collaborator in most cases. However there are situations where two approvals are required. For example, breaking changes require two approvals from TSC members. And fast-tracking a request requires two approvals. Additionally, although only one approval is strictly required, in practice, we nearly always seek a second approval when there is only one. Lastly, concerns have been raised about (perhaps unintentionally) gaming the one-approval system by suggesting a change to someone else, and then approving that change when the user submits a pull request. This resolves (or at least mitigates) that concern. Fixes: https://github.com/nodejs/node/issues/19564 --- COLLABORATOR_GUIDE.md | 7 +++---- GOVERNANCE.md | 9 ++++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index c073c07a7f0996..37ec330c08098b 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -112,10 +112,9 @@ comment that explains why the PR does not require a CI run. ### Code Reviews -All pull requests must be reviewed and accepted by a Collaborator with -sufficient expertise who is able to take full responsibility for the -change. In the case of pull requests proposed by an existing -Collaborator, an additional Collaborator is required for sign-off. +All pull requests must be reviewed and accepted by two Collaborators with +sufficient expertise who are able to take full responsibility for the change. +Approval must be from Collaborators who are not authors of the proposed changes. In some cases, it may be necessary to summon a GitHub team to a pull request for review by @-mention. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 83d4d9b50a7abe..6962ad013be93a 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -32,11 +32,10 @@ Their privileges include but are not limited to: Modifications of the contents of the nodejs/node repository are made on a collaborative basis. Anybody with a GitHub account may propose a modification via pull request and it will be considered by the project -Collaborators. All pull requests must be reviewed and accepted by a -Collaborator with sufficient expertise who is able to take full -responsibility for the change. In the case of pull requests proposed -by an existing Collaborator, an additional Collaborator is required -for sign-off. +Collaborators. All pull requests must be reviewed and accepted by two +Collaborators with sufficient expertise who are able to take full +responsibility for the change. Approval must be from Collaborators who are not +authors of the proposed changes. If one or more Collaborators oppose a proposed change, then the change cannot be accepted unless: From 1557b02aa6b1eca3d0b13733baccba377a8f3d7a Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 10 Aug 2018 15:40:45 -0700 Subject: [PATCH 2/6] squash! at least --- COLLABORATOR_GUIDE.md | 7 ++++--- GOVERNANCE.md | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index 37ec330c08098b..dced59e66131af 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -112,9 +112,10 @@ comment that explains why the PR does not require a CI run. ### Code Reviews -All pull requests must be reviewed and accepted by two Collaborators with -sufficient expertise who are able to take full responsibility for the change. -Approval must be from Collaborators who are not authors of the proposed changes. +All pull requests must be reviewed and accepted by at least two Collaborators +with sufficient expertise who are able to take full responsibility for the +change. Approval must be from Collaborators who are not authors of the proposed +changes. In some cases, it may be necessary to summon a GitHub team to a pull request for review by @-mention. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 6962ad013be93a..64b18d7dd81101 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -32,7 +32,7 @@ Their privileges include but are not limited to: Modifications of the contents of the nodejs/node repository are made on a collaborative basis. Anybody with a GitHub account may propose a modification via pull request and it will be considered by the project -Collaborators. All pull requests must be reviewed and accepted by two +Collaborators. All pull requests must be reviewed and accepted by at least two Collaborators with sufficient expertise who are able to take full responsibility for the change. Approval must be from Collaborators who are not authors of the proposed changes. From 648916353a6a315bcb38aa47d59fd3f476d4afcf Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 14 Sep 2018 15:22:52 -0700 Subject: [PATCH 3/6] squash! remove unnecessary language --- COLLABORATOR_GUIDE.md | 5 ++--- GOVERNANCE.md | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index dced59e66131af..57f07f740d335a 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -113,9 +113,8 @@ comment that explains why the PR does not require a CI run. ### Code Reviews All pull requests must be reviewed and accepted by at least two Collaborators -with sufficient expertise who are able to take full responsibility for the -change. Approval must be from Collaborators who are not authors of the proposed -changes. +who are able to take full responsibility for the change. Approval must be from +Collaborators who are not authors of the proposed changes. In some cases, it may be necessary to summon a GitHub team to a pull request for review by @-mention. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 64b18d7dd81101..b6a64d112fda2b 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -33,9 +33,8 @@ Modifications of the contents of the nodejs/node repository are made on a collaborative basis. Anybody with a GitHub account may propose a modification via pull request and it will be considered by the project Collaborators. All pull requests must be reviewed and accepted by at least two -Collaborators with sufficient expertise who are able to take full -responsibility for the change. Approval must be from Collaborators who are not -authors of the proposed changes. +Collaborators who are able to take full responsibility for the change. Approval +must be from Collaborators who are not authors of the proposed changes. If one or more Collaborators oppose a proposed change, then the change cannot be accepted unless: From a6825a6464c13ab822a352ccd4a4784b725e2c4d Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 14 Sep 2018 15:29:29 -0700 Subject: [PATCH 4/6] squash! simplify language --- COLLABORATOR_GUIDE.md | 7 ++++--- GOVERNANCE.md | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index 57f07f740d335a..1ff5a73eb628f0 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -112,9 +112,10 @@ comment that explains why the PR does not require a CI run. ### Code Reviews -All pull requests must be reviewed and accepted by at least two Collaborators -who are able to take full responsibility for the change. Approval must be from -Collaborators who are not authors of the proposed changes. +At least two Collaborators must approve a pull request before the pull request +lands. Approving a pull request indicates that the Collaborator accepts +responsibility for the change. Approval must be from Collaborators who are not +authors of the proposed changes. In some cases, it may be necessary to summon a GitHub team to a pull request for review by @-mention. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index b6a64d112fda2b..cb33eef706debe 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -32,9 +32,10 @@ Their privileges include but are not limited to: Modifications of the contents of the nodejs/node repository are made on a collaborative basis. Anybody with a GitHub account may propose a modification via pull request and it will be considered by the project -Collaborators. All pull requests must be reviewed and accepted by at least two -Collaborators who are able to take full responsibility for the change. Approval -must be from Collaborators who are not authors of the proposed changes. +Collaborators. At least two Collaborators must approve a pull request before the +pull request lands. Approving a pull request indicates that the Collaborator +accepts responsibility for the change. Approval must be from Collaborators who +are not authors of the proposed changes. If one or more Collaborators oppose a proposed change, then the change cannot be accepted unless: From da77b217c6f8d2fda34ca4041782529e350f9cea Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 14 Sep 2018 15:40:03 -0700 Subject: [PATCH 5/6] squash! escape hatch --- COLLABORATOR_GUIDE.md | 7 ++++--- GOVERNANCE.md | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index 1ff5a73eb628f0..2d9dea95ba2671 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -113,9 +113,10 @@ comment that explains why the PR does not require a CI run. ### Code Reviews At least two Collaborators must approve a pull request before the pull request -lands. Approving a pull request indicates that the Collaborator accepts -responsibility for the change. Approval must be from Collaborators who are not -authors of the proposed changes. +lands. (One Collaborator approval is enough if the pull request has been open +for more than 7 days.) Approving a pull request indicates that the Collaborator +accepts responsibility for the change. Approval must be from Collaborators who +are not authors of the proposed changes. In some cases, it may be necessary to summon a GitHub team to a pull request for review by @-mention. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index cb33eef706debe..a9711f9c2286ed 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -32,8 +32,11 @@ Their privileges include but are not limited to: Modifications of the contents of the nodejs/node repository are made on a collaborative basis. Anybody with a GitHub account may propose a modification via pull request and it will be considered by the project -Collaborators. At least two Collaborators must approve a pull request before the -pull request lands. Approving a pull request indicates that the Collaborator +Collaborators. + +At least two Collaborators must approve a pull request before the pull request +lands. (One Collaborator approval is enough if the pull request has been open +for more than 7 days.) Approving a pull request indicates that the Collaborator accepts responsibility for the change. Approval must be from Collaborators who are not authors of the proposed changes. From 130b5cafbfe4b120dde3f375c1481fbd5cd30cdb Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 14 Sep 2018 15:44:55 -0700 Subject: [PATCH 6/6] squash! unify language --- COLLABORATOR_GUIDE.md | 2 +- GOVERNANCE.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index 2d9dea95ba2671..2de267b8dfcb35 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -116,7 +116,7 @@ At least two Collaborators must approve a pull request before the pull request lands. (One Collaborator approval is enough if the pull request has been open for more than 7 days.) Approving a pull request indicates that the Collaborator accepts responsibility for the change. Approval must be from Collaborators who -are not authors of the proposed changes. +are not authors of the change. In some cases, it may be necessary to summon a GitHub team to a pull request for review by @-mention. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index a9711f9c2286ed..40ef6e7bbe5fcb 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -38,7 +38,7 @@ At least two Collaborators must approve a pull request before the pull request lands. (One Collaborator approval is enough if the pull request has been open for more than 7 days.) Approving a pull request indicates that the Collaborator accepts responsibility for the change. Approval must be from Collaborators who -are not authors of the proposed changes. +are not authors of the change. If one or more Collaborators oppose a proposed change, then the change cannot be accepted unless: