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

crypto: downgrade DEP0115 to --pending-deprecation only #23017

Closed

Conversation

addaleax
Copy link
Member

Aliases are very cheap to maintain, so an unconditional runtime
deprecation that affects existing ecosystem code is not
a good idea. This commit turns the runtime deprecation into
a --pending-deprecation one.

Fixes: #23013

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

Aliases are very cheap to maintain, so an unconditional runtime
deprecation that affects existing ecosystem code is not
a good idea. This commit turns the runtime deprecation into
a `--pending-deprecation` one.

Fixes: nodejs#23013
@nodejs-github-bot nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Sep 22, 2018
@addaleax addaleax added the deprecations Issues and PRs related to deprecations. label Sep 22, 2018
@@ -2168,12 +2168,12 @@ changes:
description: Runtime deprecation.
-->

Type: Runtime
Type: Runtime (supports [`--pending-deprecation`][])
Copy link
Member

Choose a reason for hiding this comment

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

Documentation-only

Copy link
Member Author

Choose a reason for hiding this comment

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

@targos thanks, done!

@addaleax
Copy link
Member Author

@tniessen
Copy link
Member

@addaleax Could you please update the deprecation metadata (a few lines above) accordingly? (I think it would be best to add a new entry or to update the existing with two pr-urls).

@addaleax
Copy link
Member Author

@tniessen Does this look good to you?

@tniessen
Copy link
Member

Yes, thank you!

@jasnell
Copy link
Member

jasnell commented Sep 22, 2018

hmmmm... not sure about this but won't block it.

@tniessen
Copy link
Member

@jasnell I think crypto.rng and crypto.prng are out of question, but there are still some popular packages using pseudoRandomBytes :(

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 23, 2018
@addaleax
Copy link
Member Author

Landed in 709b3b1

@addaleax addaleax closed this Sep 27, 2018
@addaleax addaleax deleted the crypto-prng-pending-deprecation branch September 27, 2018 12:45
addaleax added a commit that referenced this pull request Sep 27, 2018
Aliases are very cheap to maintain, so an unconditional runtime
deprecation that affects existing ecosystem code is not
a good idea. This commit turns the runtime deprecation into
a `--pending-deprecation` one.

Fixes: #23013

PR-URL: #23017
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Trott pushed a commit to Trott/io.js that referenced this pull request Nov 25, 2018
Make `pseudoRandomBytes` and it's aliases `prng` and `rng`
configurable to allow monkey patching.

PR-URL: nodejs#24108
Refs: nodejs#22519
Refs: nodejs#23017
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Nov 25, 2018
Make `pseudoRandomBytes` and it's aliases `prng` and `rng`
configurable to allow monkey patching.

PR-URL: #24108
Refs: #22519
Refs: #23017
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
Make `pseudoRandomBytes` and it's aliases `prng` and `rng`
configurable to allow monkey patching.

PR-URL: #24108
Refs: #22519
Refs: #23017
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
Make `pseudoRandomBytes` and it's aliases `prng` and `rng`
configurable to allow monkey patching.

PR-URL: nodejs#24108
Refs: nodejs#22519
Refs: nodejs#23017
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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. crypto Issues and PRs related to the crypto subsystem. deprecations Issues and PRs related to deprecations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

npm is not ready for Node 11
10 participants