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.

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

[v8.x] module: add builtinModules #18220

Closed

Conversation

maclover7
Copy link
Contributor

Provides list of all builtin modules in Node.

Includes modules of all types:

  • prefixed (ex: _tls_common)
  • deprecated (ex: sys)
  • regular (ex: vm)

PR-URL: #16386
Refs: #3307
Reviewed-By: Ruben Bridgewater ruben@bridgewater.de
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Colin Ihrig cjihrig@gmail.com

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

module

Provides list of all builtin modules in Node.

Includes modules of all types:
- prefixed (ex: _tls_common)
- deprecated (ex: sys)
- regular (ex: vm)

PR-URL: nodejs#16386
Refs: nodejs#3307
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@nodejs-github-bot nodejs-github-bot added module Issues and PRs related to the module subsystem. v8.x labels Jan 18, 2018

* {string[]}

A list of the names of all modules provided by Node.js. Can be used to verify
Copy link
Member

Choose a reason for hiding this comment

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

s/ / /g (2 → 1)

* {string[]}

A list of the names of all modules provided by Node.js. Can be used to verify
if a module is maintained by a third-party module or not.
Copy link
Member

Choose a reason for hiding this comment

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

Just third party; modules don’t maintain, people do.

@gibfahn
Copy link
Member

gibfahn commented Feb 19, 2018

@ljharb those issues exist on master too, so they would need to be fixed first and then backported. If you would like to raise a PR against master that would be great.

CI: https://ci.nodejs.org/job/node-test-commit/16352/
https://ci.nodejs.org/job/node-test-pull-request/13259/
https://ci.nodejs.org/job/node-test-commit/16359/

@ljharb
Copy link
Member

ljharb commented Feb 19, 2018

Sure, but I’m not sure why they couldn’t also be fixed as part of the backport.

@gibfahn
Copy link
Member

gibfahn commented Feb 19, 2018

Sure, but I’m not sure why they couldn’t also be fixed as part of the backport.

General process is not to change things as part of backports because they don't get the same level of scrutiny as PRs to master. Also it can make life more difficult going forward, for example if the PR that fixes this in master also fixes some other issues, then that will have to be manually backported to this release with the changes that have already landed removed.

gibfahn pushed a commit that referenced this pull request Feb 19, 2018
Provides list of all builtin modules in Node.

Includes modules of all types:
- prefixed (ex: _tls_common)
- deprecated (ex: sys)
- regular (ex: vm)

PR-URL: #16386
Backport-PR-URL: #18220
Refs: #3307
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@gibfahn
Copy link
Member

gibfahn commented Feb 19, 2018

Landed in c5093fc

@gibfahn gibfahn closed this Feb 19, 2018
@maclover7 maclover7 deleted the backport-16386-to-v8.x branch July 14, 2018 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module Issues and PRs related to the module subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants