From ccb22c6620c499877e333c3474c934a511b2e7d0 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 19 Mar 2018 12:02:18 -0700 Subject: [PATCH 1/6] doc: reduce CI options in COLLABORATOR_GUIDE.md --- COLLABORATOR_GUIDE.md | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index 941694ced34d1c..6ffa73c2fff617 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -192,9 +192,9 @@ All bugfixes require a test case which demonstrates the defect. The test should *fail* before the change, and *pass* after the change. All pull requests that modify executable code should also include a test case -and be subjected to continuous integration tests on the +and must be subjected to continuous integration tests on the [project CI server](https://ci.nodejs.org/). The pull request should have a CI -status indicator if possible. +status indicator. #### Useful CI Jobs @@ -203,13 +203,9 @@ is the standard CI run we do to check Pull Requests. It triggers `node-test-commit`, which runs the `build-ci` and `test-ci` targets on all supported platforms. -* [`node-test-linter`](https://ci.nodejs.org/job/node-test-linter/) -only runs the linter targets, which is useful for changes that only affect -comments or documentation. - * [`node-test-pull-request-lite`](https://ci.nodejs.org/job/node-test-pull-request-lite/) -only runs the linter job, as well as the tests on LinuxONE. Should only be used -for trivial changes that do not require being tested on all platforms. +only runs the linter job, as well as the tests on LinuxONE, which is very fast. +This is useful for changes that only affect comments or documentation. * [`citgm-smoker`](https://ci.nodejs.org/job/citgm-smoker/) uses [`CitGM`](https://github.com/nodejs/citgm) to allow you to run From 87e59badac6c7abe78f29610a1ed80ce0188f920 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 19 Mar 2018 12:04:29 -0700 Subject: [PATCH 2/6] doc: simplify COLLABORATOR_GUIDE.md instructions Remove bulleted list formatting. Only check email/name for new contributors. Change `full name` to `name` and remove italics to avoid incorrect implication that we need a "legal name". --- COLLABORATOR_GUIDE.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index 6ffa73c2fff617..882b2555d8cbd5 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -494,12 +494,11 @@ The TSC should serve as the final arbiter where required. one](https://github.com/nodejs/node/commit/b636ba8186) if unsure exactly how to format your commit messages. -Additionally: +Check PRs from new contributors to make sure the person's name and email address +are correct before merging. -* Double check PRs to make sure the person's _full name_ and email - address are correct before merging. -* All commits should be self-contained (meaning every commit should pass all - tests). This makes it much easier when bisecting to find a breaking change. +All commits should be self-contained, meaning every commit should pass all +tests. This makes it much easier when bisecting to find a breaking change. ### Using `git-node` From 2340a504de9bf4449ddc9f0da0830b7d3baad809 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 19 Mar 2018 12:13:25 -0700 Subject: [PATCH 3/6] doc: require passing CI for landing code --- COLLABORATOR_GUIDE.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index 882b2555d8cbd5..e81cd4b8fb8401 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -196,6 +196,14 @@ and must be subjected to continuous integration tests on the [project CI server](https://ci.nodejs.org/). The pull request should have a CI status indicator. +Do not land any Pull Requests without passing (green or yellow) CI runs. If you +believe any failed (red or grey) Ci sub-tasks are unrelated to the change in the +Pull Request, you may re-run the sub-task to try to see if it passes. If re-runs +of all failed sub-tasks pass, it is permissible to land the Pull Request but +only if the initial failures are believed in good faith to be unrelated to the +failure. Otherwise, reasonable steps must be taken to confirm that the change +is not resulting in an unreliable test. + #### Useful CI Jobs * [`node-test-pull-request`](https://ci.nodejs.org/job/node-test-pull-request/) From 35e25fe4fefa2756c254f6444f22c1689b91c456 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 19 Mar 2018 19:27:02 -0700 Subject: [PATCH 4/6] squash --- COLLABORATOR_GUIDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index e81cd4b8fb8401..947c66a2cee78b 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -197,7 +197,7 @@ and must be subjected to continuous integration tests on the status indicator. Do not land any Pull Requests without passing (green or yellow) CI runs. If you -believe any failed (red or grey) Ci sub-tasks are unrelated to the change in the +believe any failed (red or grey) CI sub-tasks are unrelated to the change in the Pull Request, you may re-run the sub-task to try to see if it passes. If re-runs of all failed sub-tasks pass, it is permissible to land the Pull Request but only if the initial failures are believed in good faith to be unrelated to the From 2d3047b5536140d61cc2ca8b0462769df8f806ad Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 20 Mar 2018 11:04:30 -0700 Subject: [PATCH 5/6] squash --- COLLABORATOR_GUIDE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index 947c66a2cee78b..6f6e2777e29583 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -201,8 +201,8 @@ believe any failed (red or grey) CI sub-tasks are unrelated to the change in the Pull Request, you may re-run the sub-task to try to see if it passes. If re-runs of all failed sub-tasks pass, it is permissible to land the Pull Request but only if the initial failures are believed in good faith to be unrelated to the -failure. Otherwise, reasonable steps must be taken to confirm that the change -is not resulting in an unreliable test. +changes in the Pull Request. Otherwise, reasonable steps must be taken to +confirm that the change is not resulting in an unreliable test. #### Useful CI Jobs From 62eb03fdbd55de735b7c73fcd089f5871f8a0194 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 20 Mar 2018 11:05:06 -0700 Subject: [PATCH 6/6] squash --- COLLABORATOR_GUIDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index 6f6e2777e29583..337b3d65ec3af1 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -202,7 +202,7 @@ Pull Request, you may re-run the sub-task to try to see if it passes. If re-runs of all failed sub-tasks pass, it is permissible to land the Pull Request but only if the initial failures are believed in good faith to be unrelated to the changes in the Pull Request. Otherwise, reasonable steps must be taken to -confirm that the change is not resulting in an unreliable test. +confirm that the changes are not resulting in an unreliable test. #### Useful CI Jobs