From cc22d35d625cb832cad3a8b1e3d483f47a4c41bc Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 10 Dec 2016 03:11:19 +0800 Subject: [PATCH 01/10] doc: clarify the review and landing process - Explains what "nits" stand for - Explains commit squashing - Mentions the CI run - Mentions the mandatory 48/72 hours wait - Mention GitHub's PR review feature - Fix indentation --- CONTRIBUTING.md | 52 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6f44949a31e0ca..e744ee2df4ec2b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -237,6 +237,20 @@ $ git commit --amend $ git push --force-with-lease origin my-branch ``` +When the commits in your Pull Request get landed, they will be squashed into +one commit per logical change, with metadata added to the commit message +(including links to the Pull Request, links to relevant issues, +and the names of the reviewers). The squashing will be done by someone +who has the commit access to the repository. The commit history of +your Pull Request, however, will stay intact on the Pull Request +page (as long as you don't delete your fork branch, at which point +it disappears). + +Notes: For the size of "one logical change", +[0b5191f](https://github.com/nodejs/node/commit/0b5191f15d0f311c804d542b67e2e922d98834f8) +can be a good example. It touches the implementation, the documentation, +and the tests, but is still one logical change. + **Important:** The `git push --force-with-lease` command is one of the few ways to delete history in git. Before you use it, make sure you understand the risks. If in doubt, you can always ask for guidance in the Pull Request or on @@ -245,16 +259,40 @@ If in doubt, you can always ask for guidance in the Pull Request or on Feel free to post a comment in the Pull Request to ping reviewers if you are awaiting an answer on something. +Notes: if the reviewer mentions *nits* in their comments, that means +"requests for changes that are not essential". Usually these nits are +related to coding styles or typos. It's always a good idea to pay +attention to the detail! ### Step 8: Landing -Once your Pull Request has been reviewed and approved by at least one Node.js -Collaborators (often by saying LGTM, or Looks Good To Me), and as long as -there is consensus (no objections from a Collaborator), a -Collaborator can merge the Pull Request . GitHub often shows the Pull Request as - `Closed` at this point, but don't worry. If you look at the branch you raised - your Pull Request against (probably `master`), you should see a commit with - your name on it. Congratulations and thanks for your contribution! +In order to get landed, a Pull Request needs to be reviewed and approved by +at least one Node.js Collaborator (either by saying LGTM, +which stands for "Looks Good To Me", or by using GitHub's Approve button). +GitHub's Pull Request review feature is used in this process. For more +information, check out [the video tutorial](https://www.youtube.com/watch?v=HW0RPaJqm4g) +or [the official documentation](https://help.github.com/articles/reviewing-changes-in-pull-requests/). + +Notes: After you push new changes to your branch, you need to get the +approval for these new changes again, even if GitHub shows "Approved" +because the reviwers have hit the buttons before. + +If this Pull Request touches more than the documentation, then it also +needs an all-green CI (Continuous Integration) test run. Only a +Collaborator can request a CI run. Usually one of them will do it +for you as approvals for the Pull Request come in. +If not, you can ask a Collaborator to request a CI run. + +After your Pull Request has been approved by one or more Node.js +Collaborators and has passed the CI, as long as there is consensus +(no objections from a Collaborator), it can be merged by a Collaborator. +But, if there are non-trivial changes in this Pull Request, it still +needs to wait for at least another 48 hours (72 hours on a weekend). + +GitHub often shows the Pull Request as `Closed` at this point, +but don't worry. If you look at the branch you raised your Pull Request +against (probably `master`), you should see a commit with +your name on it. Congratulations and thanks for your contribution! ## Developer's Certificate of Origin 1.1 From e8153b72a665ae54f6ddeef25cc22f7ea90f7e06 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sun, 11 Dec 2016 17:44:13 +0800 Subject: [PATCH 02/10] doc: clarify the landing process - Mention the "landed in " comment when a PR gets landed --- CONTRIBUTING.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e744ee2df4ec2b..0c9c5b4038bd00 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -289,9 +289,11 @@ Collaborators and has passed the CI, as long as there is consensus But, if there are non-trivial changes in this Pull Request, it still needs to wait for at least another 48 hours (72 hours on a weekend). -GitHub often shows the Pull Request as `Closed` at this point, -but don't worry. If you look at the branch you raised your Pull Request -against (probably `master`), you should see a commit with +When the Pull Request is landed, the person who land this will post +a comment to the Pull Request page mentioning the commit(s) in which +it is landed. GitHub often shows the Pull Request as `Closed` at this +point, but don't worry. If you look at the branch you raised your +Pull Request against (probably `master`), you should see a commit with your name on it. Congratulations and thanks for your contribution! From 0bb76bad84b720a4c89ba2ad4e7262084641d2d2 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 13 Dec 2016 13:57:21 +0800 Subject: [PATCH 03/10] doc: explain the waiting time of semver-major PRs --- CONTRIBUTING.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0c9c5b4038bd00..9a3d98a11c169a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -287,7 +287,14 @@ After your Pull Request has been approved by one or more Node.js Collaborators and has passed the CI, as long as there is consensus (no objections from a Collaborator), it can be merged by a Collaborator. But, if there are non-trivial changes in this Pull Request, it still -needs to wait for at least another 48 hours (72 hours on a weekend). +needs to wait for at least another 48 hours (72 hours on a weekend) +so that more input can be made before it gets landed. +If the Pull Request is labeled as `semver-major`, the wait could +be longer, because it introduces changes that can potentially +break other people's code in the Node.js ecosystem, and can not be +released until the major version number of Node.js increases. +All these precautions are important because Node.js is widely used, +so don't be discouraged! When the Pull Request is landed, the person who land this will post a comment to the Pull Request page mentioning the commit(s) in which From 32ab27a55384729285af906debfe0bbd6be92652 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 14 Dec 2016 17:37:40 +0800 Subject: [PATCH 04/10] doc: put the notes and details in the back --- CONTRIBUTING.md | 103 ++++++++++++++++++++++++++++-------------------- 1 file changed, 60 insertions(+), 43 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9a3d98a11c169a..213626a20b202e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -237,72 +237,89 @@ $ git commit --amend $ git push --force-with-lease origin my-branch ``` -When the commits in your Pull Request get landed, they will be squashed into -one commit per logical change, with metadata added to the commit message -(including links to the Pull Request, links to relevant issues, -and the names of the reviewers). The squashing will be done by someone -who has the commit access to the repository. The commit history of -your Pull Request, however, will stay intact on the Pull Request -page (as long as you don't delete your fork branch, at which point -it disappears). - -Notes: For the size of "one logical change", -[0b5191f](https://github.com/nodejs/node/commit/0b5191f15d0f311c804d542b67e2e922d98834f8) -can be a good example. It touches the implementation, the documentation, -and the tests, but is still one logical change. - **Important:** The `git push --force-with-lease` command is one of the few ways to delete history in git. Before you use it, make sure you understand the risks. If in doubt, you can always ask for guidance in the Pull Request or on [IRC in the #node-dev channel](https://webchat.freenode.net?channels=node-dev&uio=d4). Feel free to post a comment in the Pull Request to ping reviewers if you are -awaiting an answer on something. +awaiting an answer on something. If you encounter words or acronyms that +seem unfamiliar, check out the [glossary](https://sites.google.com/a/chromium.org/dev/glossary). -Notes: if the reviewer mentions *nits* in their comments, that means -"requests for changes that are not essential". Usually these nits are -related to coding styles or typos. It's always a good idea to pay -attention to the detail! +Note that your commits usually get squashed when they are landed (see +notes about [commit squashing](#commit-squashing)). ### Step 8: Landing -In order to get landed, a Pull Request needs to be reviewed and approved by -at least one Node.js Collaborator (either by saying LGTM, -which stands for "Looks Good To Me", or by using GitHub's Approve button). -GitHub's Pull Request review feature is used in this process. For more -information, check out [the video tutorial](https://www.youtube.com/watch?v=HW0RPaJqm4g) +In order to get landed, a Pull Request needs to be reviewed and +[approved](#getting-approvals-for-your-pull-request) by +at least one Node.js Collaborator. If this Pull Request touches +more than the documentation, then it also needs an all-green +[CI (Continuous Integration) test run](#ci-testing). After that, +as long as there is no objections from a Collaborator, this +Pull Request can be merged. If you find your Pull Request waiting +longer than you expect, see +[notes about the waiting time](#waiting-until-the-pull-request-gets-landed). + +When the Pull Request is landed, the person who land this will post +a comment to the Pull Request page mentioning the commit(s) in which +it is landed. GitHub often shows the Pull Request as `Closed` at this +point, but don't worry. If you look at the branch you raised your +Pull Request against (probably `master`), you should see a commit with +your name on it. Congratulations and thanks for your contribution! + +## Additional Notes + +### Commit Squashing + +When the commits in your Pull Request get landed, they will be squashed +into one commit per logical change, with metadata added to the commit +message (including links to the Pull Request, links to relevant issues, +and the names of the reviewers). As long as you don't delete +your fork branch, the commit history of your Pull Request will stay +intact on the Pull Request page. + +For the size of "one logical change", +[0b5191f](https://github.com/nodejs/node/commit/0b5191f15d0f311c804d542b67e2e922d98834f8) +can be a good example. It touches the implementation, the documentation, +and the tests, but is still one logical change. + +### Getting Approvals for Your Pull Request + +The approval can be made either by saying LGTM, which stands for +"Looks Good To Me", or by using GitHub's Approve button. +GitHub's Pull Request review feature is used during the process. +For more information, check out +[the video tutorial](https://www.youtube.com/watch?v=HW0RPaJqm4g) or [the official documentation](https://help.github.com/articles/reviewing-changes-in-pull-requests/). -Notes: After you push new changes to your branch, you need to get the +After you push new changes to your branch, you need to get the approval for these new changes again, even if GitHub shows "Approved" because the reviwers have hit the buttons before. -If this Pull Request touches more than the documentation, then it also -needs an all-green CI (Continuous Integration) test run. Only a -Collaborator can request a CI run. Usually one of them will do it +### CI Testing + +If your Pull Request contains executable code, it needs to be tested +to make sure the code works on the platforms that Node.js +supports. This is done by running the code through the CI system. + +Only a Collaborator can request a CI run. Usually one of them will do it for you as approvals for the Pull Request come in. If not, you can ask a Collaborator to request a CI run. -After your Pull Request has been approved by one or more Node.js -Collaborators and has passed the CI, as long as there is consensus -(no objections from a Collaborator), it can be merged by a Collaborator. -But, if there are non-trivial changes in this Pull Request, it still -needs to wait for at least another 48 hours (72 hours on a weekend) +### Waiting Until the Pull Request Gets Landed + +If there are non-trivial changes in this Pull Request, after it +gets approved and passes the CI, it still needs to wait +for at least another 48 hours (72 hours on a weekend) so that more input can be made before it gets landed. + If the Pull Request is labeled as `semver-major`, the wait could -be longer, because it introduces changes that can potentially -break other people's code in the Node.js ecosystem, and can not be -released until the major version number of Node.js increases. +be longer, since it can not be released until the major +version number of Node.js increases. All these precautions are important because Node.js is widely used, so don't be discouraged! -When the Pull Request is landed, the person who land this will post -a comment to the Pull Request page mentioning the commit(s) in which -it is landed. GitHub often shows the Pull Request as `Closed` at this -point, but don't worry. If you look at the branch you raised your -Pull Request against (probably `master`), you should see a commit with -your name on it. Congratulations and thanks for your contribution! - ## Developer's Certificate of Origin 1.1 From 9bda4a45baa7c7c093c52e42d70a9521395c4182 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 15 Dec 2016 17:58:19 +0800 Subject: [PATCH 05/10] doc: improve the clarification --- CONTRIBUTING.md | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 213626a20b202e..dfa4cfed16353b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -253,15 +253,15 @@ notes about [commit squashing](#commit-squashing)). In order to get landed, a Pull Request needs to be reviewed and [approved](#getting-approvals-for-your-pull-request) by -at least one Node.js Collaborator. If this Pull Request touches -more than the documentation, then it also needs an all-green +at least one Node.js Collaborator. If the Pull Request touches +more than the documentation, then it also need to pass a [CI (Continuous Integration) test run](#ci-testing). After that, -as long as there is no objections from a Collaborator, this +as long as there are no objections from a Collaborator, the Pull Request can be merged. If you find your Pull Request waiting -longer than you expect, see +longer than you expect, see the [notes about the waiting time](#waiting-until-the-pull-request-gets-landed). -When the Pull Request is landed, the person who land this will post +When a collaborator lands your Pull Request, they will post a comment to the Pull Request page mentioning the commit(s) in which it is landed. GitHub often shows the Pull Request as `Closed` at this point, but don't worry. If you look at the branch you raised your @@ -295,7 +295,7 @@ or [the official documentation](https://help.github.com/articles/reviewing-chang After you push new changes to your branch, you need to get the approval for these new changes again, even if GitHub shows "Approved" -because the reviwers have hit the buttons before. +because the reviewers have hit the buttons before. ### CI Testing @@ -309,14 +309,12 @@ If not, you can ask a Collaborator to request a CI run. ### Waiting Until the Pull Request Gets Landed -If there are non-trivial changes in this Pull Request, after it -gets approved and passes the CI, it still needs to wait -for at least another 48 hours (72 hours on a weekend) -so that more input can be made before it gets landed. - -If the Pull Request is labeled as `semver-major`, the wait could -be longer, since it can not be released until the major -version number of Node.js increases. +After the Pull Request gets approved and passes the CI, +it still needs to wait for at least another 48 hours (72 +hours on a weekend) so that everyone has a chance to +weigh in. If the changes are trivial, collaborators may +decide it doesn't need to wait. But if the Pull Request is +labeled as `semver-major`, it can take longer than the minimum. All these precautions are important because Node.js is widely used, so don't be discouraged! From dbd83ef6ce2a2ceefe4322d01b4ba81f8dfd37ba Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 15 Dec 2016 20:58:04 +0800 Subject: [PATCH 06/10] doc: more fixes about the process --- CONTRIBUTING.md | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dfa4cfed16353b..4d231e210e64dc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -244,9 +244,10 @@ If in doubt, you can always ask for guidance in the Pull Request or on Feel free to post a comment in the Pull Request to ping reviewers if you are awaiting an answer on something. If you encounter words or acronyms that -seem unfamiliar, check out the [glossary](https://sites.google.com/a/chromium.org/dev/glossary). +seem unfamiliar, check out this +[glossary](https://sites.google.com/a/chromium.org/dev/glossary). -Note that your commits usually get squashed when they are landed (see +Note that multiple commits often get squashed when they are landed (see the notes about [commit squashing](#commit-squashing)). ### Step 8: Landing @@ -262,8 +263,8 @@ longer than you expect, see the [notes about the waiting time](#waiting-until-the-pull-request-gets-landed). When a collaborator lands your Pull Request, they will post -a comment to the Pull Request page mentioning the commit(s) in which -it is landed. GitHub often shows the Pull Request as `Closed` at this +a comment to the Pull Request page mentioning the commit(s) it +landed as. GitHub often shows the Pull Request as `Closed` at this point, but don't worry. If you look at the branch you raised your Pull Request against (probably `master`), you should see a commit with your name on it. Congratulations and thanks for your contribution! @@ -286,14 +287,14 @@ and the tests, but is still one logical change. ### Getting Approvals for Your Pull Request -The approval can be made either by saying LGTM, which stands for +A Pull Request is approved either by saying LGTM, which stands for "Looks Good To Me", or by using GitHub's Approve button. -GitHub's Pull Request review feature is used during the process. +GitHub's Pull Request review feature can be used during the process. For more information, check out [the video tutorial](https://www.youtube.com/watch?v=HW0RPaJqm4g) or [the official documentation](https://help.github.com/articles/reviewing-changes-in-pull-requests/). -After you push new changes to your branch, you need to get the +After you push new changes to your branch, you need to get approval for these new changes again, even if GitHub shows "Approved" because the reviewers have hit the buttons before. @@ -313,11 +314,17 @@ After the Pull Request gets approved and passes the CI, it still needs to wait for at least another 48 hours (72 hours on a weekend) so that everyone has a chance to weigh in. If the changes are trivial, collaborators may -decide it doesn't need to wait. But if the Pull Request is -labeled as `semver-major`, it can take longer than the minimum. +decide it doesn't need to wait. A Pull Request may well take +longer to be merged in. All these precautions are important because Node.js is widely used, so don't be discouraged! +### Check Out the Collaborator's Guide + +If you want to know more about the code review and the landing process, +you can take a look at the +[collaborator's guide](https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md). + ## Developer's Certificate of Origin 1.1 From 96e0189ca2fdb7df212746754a415ce06478b7ec Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 16 Dec 2016 11:34:17 +0800 Subject: [PATCH 07/10] doc: amends the contributing guide --- CONTRIBUTING.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4d231e210e64dc..6de8fb4638268b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -283,7 +283,8 @@ intact on the Pull Request page. For the size of "one logical change", [0b5191f](https://github.com/nodejs/node/commit/0b5191f15d0f311c804d542b67e2e922d98834f8) can be a good example. It touches the implementation, the documentation, -and the tests, but is still one logical change. +and the tests, but is still one logical change. In general, the tests should +always pass when each individual commit lands on the master branch. ### Getting Approvals for Your Pull Request @@ -310,14 +311,13 @@ If not, you can ask a Collaborator to request a CI run. ### Waiting Until the Pull Request Gets Landed -After the Pull Request gets approved and passes the CI, -it still needs to wait for at least another 48 hours (72 -hours on a weekend) so that everyone has a chance to -weigh in. If the changes are trivial, collaborators may -decide it doesn't need to wait. A Pull Request may well take -longer to be merged in. -All these precautions are important because Node.js is widely used, -so don't be discouraged! +A Pull Request needs to stay open for at least 48 hours (72 hours on a +weekend) from when it is submitted, even after it gets approved and +passes the CI. This is to make sure that everyone has a chance to +weigh in. If the changes are trivial, collaborators may decide it +doesn't need to wait. A Pull Request may well take longer to be +merged in. All these precautions are important because Node.js is +widely used, so don't be discouraged! ### Check Out the Collaborator's Guide From 246f52aa0cd626dfc3ea2e185b02fc34e474c17a Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 16 Dec 2016 22:20:36 +0800 Subject: [PATCH 08/10] doc: testing docs, PR history preservation --- CONTRIBUTING.md | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6de8fb4638268b..9d0d918c7a98a1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -254,12 +254,11 @@ notes about [commit squashing](#commit-squashing)). In order to get landed, a Pull Request needs to be reviewed and [approved](#getting-approvals-for-your-pull-request) by -at least one Node.js Collaborator. If the Pull Request touches -more than the documentation, then it also need to pass a -[CI (Continuous Integration) test run](#ci-testing). After that, -as long as there are no objections from a Collaborator, the -Pull Request can be merged. If you find your Pull Request waiting -longer than you expect, see the +at least one Node.js Collaborator and pass a +[CI (Continuous Integration) test run](#ci-testing). +After that, as long as there are no objections +from a Collaborator, the Pull Request can be merged. If you find your +Pull Request waiting longer than you expect, see the [notes about the waiting time](#waiting-until-the-pull-request-gets-landed). When a collaborator lands your Pull Request, they will post @@ -276,9 +275,8 @@ your name on it. Congratulations and thanks for your contribution! When the commits in your Pull Request get landed, they will be squashed into one commit per logical change, with metadata added to the commit message (including links to the Pull Request, links to relevant issues, -and the names of the reviewers). As long as you don't delete -your fork branch, the commit history of your Pull Request will stay -intact on the Pull Request page. +and the names of the reviewers). The commit history of your Pull Request, +however, will stay intact on the Pull Request page. For the size of "one logical change", [0b5191f](https://github.com/nodejs/node/commit/0b5191f15d0f311c804d542b67e2e922d98834f8) @@ -301,8 +299,8 @@ because the reviewers have hit the buttons before. ### CI Testing -If your Pull Request contains executable code, it needs to be tested -to make sure the code works on the platforms that Node.js +Every Pull Request needs to be tested +to make sure the it works on the platforms that Node.js supports. This is done by running the code through the CI system. Only a Collaborator can request a CI run. Usually one of them will do it From e9c53f3ea3e0ad89364e242497e808481bf9fac8 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 20 Dec 2016 17:48:56 +0800 Subject: [PATCH 09/10] doc: fix typo --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9d0d918c7a98a1..fa964dd7959820 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -300,7 +300,7 @@ because the reviewers have hit the buttons before. ### CI Testing Every Pull Request needs to be tested -to make sure the it works on the platforms that Node.js +to make sure that it works on the platforms that Node.js supports. This is done by running the code through the CI system. Only a Collaborator can request a CI run. Usually one of them will do it From 41e4e935ec5fbc640ce6ff368d01f70cc1d19e79 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 22 Dec 2016 14:24:08 +0800 Subject: [PATCH 10/10] doc: trim trailling spaces --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fa964dd7959820..360a849573f40b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -287,7 +287,7 @@ always pass when each individual commit lands on the master branch. ### Getting Approvals for Your Pull Request A Pull Request is approved either by saying LGTM, which stands for -"Looks Good To Me", or by using GitHub's Approve button. +"Looks Good To Me", or by using GitHub's Approve button. GitHub's Pull Request review feature can be used during the process. For more information, check out [the video tutorial](https://www.youtube.com/watch?v=HW0RPaJqm4g) @@ -311,7 +311,7 @@ If not, you can ask a Collaborator to request a CI run. A Pull Request needs to stay open for at least 48 hours (72 hours on a weekend) from when it is submitted, even after it gets approved and -passes the CI. This is to make sure that everyone has a chance to +passes the CI. This is to make sure that everyone has a chance to weigh in. If the changes are trivial, collaborators may decide it doesn't need to wait. A Pull Request may well take longer to be merged in. All these precautions are important because Node.js is