From ec3ed97bb420ab4bcd25a79bfa52eebcb21df5aa Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 18 Jun 2024 13:54:26 +0000 Subject: [PATCH 1/4] doc: add additional guidance for PRs to deps - add additional guidance based in discussion related to recent PR to dependency and discussion within the security-wg slack channel. Refs: https://github.com/nodejs/security-wg/issues/1329 Signed-off-by: Michael Dawson --- doc/contributing/maintaining/maintaining-dependencies.md | 5 +++++ doc/contributing/pull-requests.md | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/doc/contributing/maintaining/maintaining-dependencies.md b/doc/contributing/maintaining/maintaining-dependencies.md index e21a6409b3c896..e735fe29e5706d 100644 --- a/doc/contributing/maintaining/maintaining-dependencies.md +++ b/doc/contributing/maintaining/maintaining-dependencies.md @@ -144,6 +144,11 @@ the corresponding script in `tools/update-deps`. [npm-cli-bot](https://github.com/npm/cli/blob/latest/.github/workflows/create-node-pr.yml) takes care of npm update, it is maintained by the npm team. +PRs for manual dependency updates should only be accepted if +the update cannot be generated by the automated tooling, +the reason is clearly documented and either the PR is +reviewed in detail or it is from an existing collaborator. + ## Dependency list ### acorn diff --git a/doc/contributing/pull-requests.md b/doc/contributing/pull-requests.md index 295e9d3695c47e..1dc4ea48cf1822 100644 --- a/doc/contributing/pull-requests.md +++ b/doc/contributing/pull-requests.md @@ -525,6 +525,13 @@ to fail on specific platforms or for so-called "flaky" tests to fail ("be red"). It is vital to visually inspect the results of all failed ("red") tests to determine whether the failure was caused by the changes in the pull request. +### Dependencies + +Ideally pull requests for dependencies should be generated by automation. +Pay special attention to pull requests for dependencies which have not +been automatically generated and follow the guidance in +[Maintaining Dependencies](https://github.com/nodejs/node/blob/main/doc/contributing/maintaining/maintaining-dependencies.md#updating-dependencies). + ## Notes ### Commit squashing From be913d09d69bf71c0d0348feb7ee2ca18bfc0961 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Wed, 19 Jun 2024 15:05:35 -0400 Subject: [PATCH 2/4] address comments --- doc/contributing/maintaining/maintaining-dependencies.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/contributing/maintaining/maintaining-dependencies.md b/doc/contributing/maintaining/maintaining-dependencies.md index e735fe29e5706d..dbd8019c4a52a8 100644 --- a/doc/contributing/maintaining/maintaining-dependencies.md +++ b/doc/contributing/maintaining/maintaining-dependencies.md @@ -149,6 +149,11 @@ the update cannot be generated by the automated tooling, the reason is clearly documented and either the PR is reviewed in detail or it is from an existing collaborator. +In general updates to dependencies should only be accepted +if they have already landed in the upstream. This avoids +the project having to float patches for a long time and +ensures that tooling can generate updates automatically. + ## Dependency list ### acorn From 748e8ca78921150ec93acc08bbd55ad4cd185690 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Wed, 19 Jun 2024 15:09:51 -0400 Subject: [PATCH 3/4] Update maintaining-dependencies.md --- doc/contributing/maintaining/maintaining-dependencies.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/contributing/maintaining/maintaining-dependencies.md b/doc/contributing/maintaining/maintaining-dependencies.md index dbd8019c4a52a8..d1bfc7dcaf6755 100644 --- a/doc/contributing/maintaining/maintaining-dependencies.md +++ b/doc/contributing/maintaining/maintaining-dependencies.md @@ -150,7 +150,8 @@ the reason is clearly documented and either the PR is reviewed in detail or it is from an existing collaborator. In general updates to dependencies should only be accepted -if they have already landed in the upstream. This avoids +if they have already landed in the upstream. The TSC may +grant an exception on a case-by-case basis. This avoids the project having to float patches for a long time and ensures that tooling can generate updates automatically. From 12b9007d53fa4336b2c9bdaaa596a3e65d5dc32b Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Wed, 19 Jun 2024 22:17:55 +0000 Subject: [PATCH 4/4] squash: address comments Signed-off-by: Michael Dawson --- doc/contributing/collaborator-guide.md | 5 +++++ doc/contributing/pull-requests.md | 7 ------- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/doc/contributing/collaborator-guide.md b/doc/contributing/collaborator-guide.md index 43adfbb8a272ba..4eea65e8c061f4 100644 --- a/doc/contributing/collaborator-guide.md +++ b/doc/contributing/collaborator-guide.md @@ -127,6 +127,11 @@ for the change. Approval must be from collaborators who are not authors of the change. +Ideally pull requests for dependencies should be generated by automation. +Pay special attention to pull requests for dependencies which have not +been automatically generated and follow the guidance in +[Maintaining Dependencies](https://github.com/nodejs/node/blob/main/doc/contributing/maintaining/maintaining-dependencies.md#updating-dependencies). + In some cases, it might be necessary to summon a GitHub team to a pull request for review by @-mention. See [Who to CC in the issue tracker](#who-to-cc-in-the-issue-tracker). diff --git a/doc/contributing/pull-requests.md b/doc/contributing/pull-requests.md index 1dc4ea48cf1822..295e9d3695c47e 100644 --- a/doc/contributing/pull-requests.md +++ b/doc/contributing/pull-requests.md @@ -525,13 +525,6 @@ to fail on specific platforms or for so-called "flaky" tests to fail ("be red"). It is vital to visually inspect the results of all failed ("red") tests to determine whether the failure was caused by the changes in the pull request. -### Dependencies - -Ideally pull requests for dependencies should be generated by automation. -Pay special attention to pull requests for dependencies which have not -been automatically generated and follow the guidance in -[Maintaining Dependencies](https://github.com/nodejs/node/blob/main/doc/contributing/maintaining/maintaining-dependencies.md#updating-dependencies). - ## Notes ### Commit squashing