★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/23017Nouvelle recherche | Portail wanayoo
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

@addaleax addaleax commented Sep 22, 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

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
targos
targos approved these changes Sep 22, 2018
@@ -2168,12 +2168,12 @@ changes:
description: Runtime deprecation.
-->
Type: Runtime
Type: Runtime (supports [`--pending-deprecation`][])
Copy link
Member

@targos targos Sep 22, 2018

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

@addaleax addaleax Sep 22, 2018

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

@addaleax addaleax commented Sep 22, 2018

lpinca
lpinca approved these changes Sep 22, 2018
@tniessen
Copy link
Member

@tniessen tniessen commented Sep 22, 2018

@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

@addaleax addaleax commented Sep 22, 2018

@tniessen Does this look good to you?

@tniessen
Copy link
Member

@tniessen tniessen commented Sep 22, 2018

Yes, thank you!

refack
refack approved these changes Sep 22, 2018
@jasnell
Copy link
Member

@jasnell jasnell commented Sep 22, 2018

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

@tniessen
Copy link
Member

@tniessen tniessen commented Sep 22, 2018

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

@targos targos added this to Don't land (for now) in v10.x Sep 23, 2018
@targos targos moved this from Don't land (for now) to Don't land (ever) in v10.x Sep 23, 2018
@addaleax
Copy link
Member Author

@addaleax addaleax commented Sep 27, 2018

Landed in 709b3b1

@addaleax addaleax closed this Sep 27, 2018
v10.x automation moved this from Don't land (ever) to Closed PRs Sep 27, 2018
@addaleax addaleax deleted the crypto-prng-pending-deprecation branch Sep 27, 2018
addaleax added a commit that referenced this issue 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>
@targos targos moved this from Closed PRs to Don't land (ever) in v10.x Oct 5, 2018
Trott added a commit to Trott/io.js that referenced this issue 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 added a commit that referenced this issue 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 added a commit that referenced this issue 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 added a commit to refack/node that referenced this issue 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
Projects
No open projects
v10.x
  
Don't land (ever)
Linked issues

Successfully merging this pull request may close these issues.

10 participants