Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

doc: add additional guidance for PRs to deps #53499

Closed
wants to merge 4 commits into from
Closed

doc: add additional guidance for PRs to deps #53499

wants to merge 4 commits into from

Conversation

mhdawson
Copy link
Member

  • add additional guidance based in discussion related to recent PR to dependency and discussion within the security-wg slack channel.

Refs: nodejs/security-wg#1329

@mhdawson
doc: add additional guidance for PRs to deps
ec3ed97 
- add additional guidance based in discussion related
  to recent PR to dependency and discussion within the
  security-wg slack channel.

Refs: nodejs/security-wg#1329

Signed-off-by: Michael Dawson <[email protected]>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jun 18, 2024
This was referenced Jun 18, 2024
Open
Closed
RedYetiDev
RedYetiDev reviewed Jun 18, 2024
View reviewed changes
Copy link
Member

@RedYetiDev RedYetiDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nitpick, but as a triage member, this review isn't blocking nor approving.

doc/contributing/maintaining/maintaining-dependencies.md Show resolved Hide resolved
aduh95
aduh95 reviewed Jun 19, 2024
View reviewed changes
doc/contributing/maintaining/maintaining-dependencies.md
Comment on lines +147 to +150
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a note about only accepting changes that have landed upstream, and "the TSC may grant exception on a case-by-case basis"? I think it's already more or less the policy we're currently organically following, so IMO it'd make sense explicitly state it out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aduh95 that is a good idea.

Copy link
Member Author

@mhdawson mhdawson Jun 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aduh95 added some wording along those lines.

@mhdawson
squash: address comments
12b9007 
Signed-off-by: Michael Dawson <[email protected]>
@marco-ippolito marco-ippolito added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 20, 2024
mhdawson added a commit that referenced this pull request Jun 20, 2024
@mhdawson
doc: add additional guidance for PRs to deps
53e9106 
- add additional guidance based in discussion related
  to recent PR to dependency and discussion within the
  security-wg slack channel.

Refs: nodejs/security-wg#1329

Signed-off-by: Michael Dawson <[email protected]>
PR-URL: #53499
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@mhdawson
Copy link
Member Author

Landed in 53e9106

@mhdawson mhdawson closed this Jun 20, 2024
EliphazBouye pushed a commit to EliphazBouye/node that referenced this pull request Jun 20, 2024
@mhdawson @EliphazBouye
doc: add additional guidance for PRs to deps
aee48f6 
- add additional guidance based in discussion related
  to recent PR to dependency and discussion within the
  security-wg slack channel.

Refs: nodejs/security-wg#1329

Signed-off-by: Michael Dawson <[email protected]>
PR-URL: nodejs#53499
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this pull request Jun 21, 2024
@mhdawson @targos
doc: add additional guidance for PRs to deps
30858ec 
- add additional guidance based in discussion related
  to recent PR to dependency and discussion within the
  security-wg slack channel.

Refs: nodejs/security-wg#1329

Signed-off-by: Michael Dawson <[email protected]>
PR-URL: #53499
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
@mhdawson @bmeck
doc: add additional guidance for PRs to deps
734ebf9 
- add additional guidance based in discussion related
  to recent PR to dependency and discussion within the
  security-wg slack channel.

Refs: nodejs/security-wg#1329

Signed-off-by: Michael Dawson <[email protected]>
PR-URL: nodejs#53499
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@targos targos mentioned this pull request Jun 25, 2024
Merged
@Karliz24 Karliz24 mentioned this pull request Jul 9, 2024
Open
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
@mhdawson @marco-ippolito
doc: add additional guidance for PRs to deps
e7f9980 
- add additional guidance based in discussion related
  to recent PR to dependency and discussion within the
  security-wg slack channel.

Refs: nodejs/security-wg#1329

Signed-off-by: Michael Dawson <[email protected]>
PR-URL: #53499
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
@mhdawson @marco-ippolito
doc: add additional guidance for PRs to deps
c13600f 
- add additional guidance based in discussion related
  to recent PR to dependency and discussion within the
  security-wg slack channel.

Refs: nodejs/security-wg#1329

Signed-off-by: Michael Dawson <[email protected]>
PR-URL: #53499
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@marco-ippolito marco-ippolito mentioned this pull request Jul 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RedYetiDev RedYetiDev RedYetiDev left review comments

@aduh95 aduh95 aduh95 left review comments

@marco-ippolito marco-ippolito marco-ippolito approved these changes

@RafaelGSS RafaelGSS RafaelGSS approved these changes

@legendecas legendecas legendecas approved these changes

@richardlau richardlau richardlau approved these changes

@UlisesGascon UlisesGascon UlisesGascon approved these changes

@targos targos targos approved these changes

@lpinca lpinca lpinca approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.