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

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ec3ed97
doc: add additional guidance for PRs to deps
mhdawson Jun 18, 2024
be913d0
address comments
mhdawson Jun 19, 2024
748e8ca
Update maintaining-dependencies.md
mhdawson Jun 19, 2024
12b9007
squash: address comments
mhdawson Jun 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/contributing/maintaining/maintaining-dependencies.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
mhdawson marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +147 to +150
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.


## Dependency list

### acorn
Expand Down
7 changes: 7 additions & 0 deletions doc/contributing/pull-requests.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).

richardlau marked this conversation as resolved.
Show resolved Hide resolved
## Notes

### Commit squashing
Expand Down
Loading