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

process: remove undocumented now argument from emitWarning() #31643

Merged
merged 1 commit into from
Feb 8, 2020

Conversation

Trott
Copy link
Member

@Trott Trott commented Feb 5, 2020

process.emitWarning() "now" option is undocumented and a Boolean trap.
Remove it before people start adopting it.

We only need it in one place internally. Replace it with an
internal-only emitWarningSync() function.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the process Issues and PRs related to the process subsystem. label Feb 5, 2020
@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member Author

Trott commented Feb 5, 2020

I hope nothing in the ecosystem uses this.

Incidentally, it was added in 16689e3 but it looks like we don't emit the warnings that it was added for anymore. The ES Modules loader is the only thing that needs it, it would seem.

@Trott
Copy link
Member Author

Trott commented Feb 5, 2020

@Trott Trott added the semver-major PRs that contain breaking changes and should be released in the next major version. label Feb 5, 2020
@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member Author

Trott commented Feb 7, 2020

@nodejs/tsc This could use some reviews.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 7, 2020

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

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

RSLGTM if CITGM is green

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 7, 2020
process.emitWarning() "now" option is undocumented and a Boolean trap.
Remove it before people start adopting it.

We only need it in one place internally. Replace it with an
internal-only emitWarningSync() function.

PR-URL: nodejs#31643
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@Trott Trott merged commit 8c18e91 into nodejs:master Feb 8, 2020
@Trott Trott deleted the hi-2 branch February 8, 2020 01:39
codebytere added a commit to electron/electron that referenced this pull request Feb 12, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 15, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 18, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 21, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 21, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 24, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 24, 2020
* chore: bump node in DEPS to v12.16.0

* Fixup asar support setup patch

nodejs/node#30862

* Fixup InternalCallbackScope patch

nodejs/node#30236

* Fixup GN buildfiles patch

nodejs/node#30755

* Fixup low-level hooks patch

nodejs/node#30466

* Fixup globals require patch

nodejs/node#31643

* Fixup process stream patch

nodejs/node#30862

* Fixup js2c modification patch

nodejs/node#30755

* Fixup internal fs override patch

nodejs/node#30610

* Fixup context-aware warn patch

nodejs/node#30336

* Fixup Node.js with ltcg config

nodejs/node#29388

* Fixup oaepLabel patch

nodejs/node#30917

* Remove redundant ESM test patch

nodejs/node#30997

* Remove redundant cli flag patch

nodejs/node#30466

* Update filenames.json

* Remove macro generation in GN build files

nodejs/node#30755

* Fix some compilation errors upstream

* Add uvwasi to deps

nodejs/node#30258

* Fix BoringSSL incompatibilities

* Fixup linked module patch

nodejs/node#30274

* Add missing sources to GN uv build

libuv/libuv#2347

* Patch some uvwasi incompatibilities

* chore: bump Node.js to v12.6.1

* Remove mark_arraybuffer_as_untransferable.patch

nodejs/node#30549

* Fix uvwasi build failure on win

* Fixup --perf-prof cli option error

* Fixup early cjs module loading

* fix: initialize diagnostics properly

nodejs/node#30025

* Disable new esm syntax specs

nodejs/node#30219

* Fixup v8 weakref hook spec

nodejs/node#29874

* Fix async context timer issue

* Disable monkey-patch-main spec

It relies on nodejs/node#29777, and we don't
override prepareStackTrace.

* Disable new tls specs

nodejs/node#23188

We don't support much of TLS owing to schisms between BoringSSL and
OpenSSL.

Co-authored-by: Shelley Vohr <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. process Issues and PRs related to the process subsystem. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants