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: update guidance for adding new modules #44576

Closed
wants to merge 10 commits into from
Closed

doc: update guidance for adding new modules #44576

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b4b1ced
doc: update guidance for adding new modules
mhdawson Sep 8, 2022
9b3ae9a
squash:fixup
mhdawson Sep 8, 2022
e82833b
Update doc/contributing/collaborator-guide.md
Trott Sep 8, 2022
347453e
Update doc/contributing/collaborator-guide.md
Trott Sep 8, 2022
621a8b0
Update doc/contributing/collaborator-guide.md
Trott Sep 8, 2022
d129114
Update doc/contributing/collaborator-guide.md
Trott Sep 8, 2022
af6919e
Update doc/contributing/collaborator-guide.md
Trott Sep 8, 2022
a9fe2fe
Update doc/contributing/collaborator-guide.md
mhdawson Sep 9, 2022
0b19d4e
Update doc/contributing/collaborator-guide.md
mhdawson Sep 9, 2022
c524819
Update doc/contributing/collaborator-guide.md
mhdawson Sep 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 14 additions & 6 deletions doc/contributing/collaborator-guide.md
View file
Open in desktop
Expand Up @@ -392,13 +392,21 @@ metadata. Raise a pull request like any other change.

Treat commits that introduce new core modules with extra care.

Check if the module's name conflicts with an existing ecosystem module. If it
does, choose a different name unless the module owner has agreed in writing to
transfer it.
New modules must only be added with the `node:` prefix.
Copy link
Member

Choose a reason for hiding this comment

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

It seems very unfortunate to reify this when it seems a clear decision was not, in fact, made. Why can't this go to a new vote where the intention is made clear from the start?

Copy link
Member Author

Choose a reason for hiding this comment

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

If anybody on the @nodejs/tsc feels there is not consensus in the TSC on this point, and we should have a vote, either chime in on the issue, or contact me privately if you prefer and I'll move for a vote. Otherwise we'll assume consensus unless I hear from somebody by the next TSC meeting. I'm happy to have a vote if needed but our governance only requires one if we can't reach consensus among the TSC first.


If the new module name is free, register a placeholder in the module registry as
soon as possible. Link to the pull request that introduces the new core module
in the placeholder's `README`.
When adding promises to an existing API, add `/promises`
(`inspector/promises`, etc.). Apply the `semver-major` label to the addition.
Copy link
Member

@ljharb ljharb Sep 13, 2022 •
edited

Choose a reason for hiding this comment

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

if node added a semver-major change that prevented any slashed requires/imports from core module names, then this category would no longer be semver-major after that.

(edit: oops, this was mentioned here: #44576 (comment) )

Copy link
Member

Choose a reason for hiding this comment

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

I created #44651 for that.


If the new module name is free in npm, register
a placeholder in the module registry as soon as possible. Link to the pull
request that introduces the new core module in the placeholder's `README`.

If the module name is not free and the module is
not widely used, contact the owner to see if they would be willing to transfer
it to the project.

targos marked this conversation as resolved.
Show resolved Hide resolved
We register a placeholder without the `node:` prefix whenever
possible to avoid confusion and typosquatting attacks.

For pull requests introducing new core modules:

Expand Down