Deprecate mixed async crypto #2170
Conversation
Fishrock123
added
the
semver-minor
|
Possibly waiting on #1704 |
|
@thefourtheye you had a suggestion about the deprecation rewrite, but I can't seem to find it in the mess that is #632. Could you elaborate again? |
| exports.rngSync = randomBytes; | ||
| exports.prng = util.deprecate(randomBytes, | ||
| '`crypto.prng()` is deprecated. ' + | ||
| 'Please use `crypto.rng()` instead.'); |
There was a problem hiding this comment.
This should probably point to crypto.rngSync.
|
updated with just the patch. running the tests locally right now |
|
shouldn't it say, "used asynchronously"? |
|
your patch gave me lint errors :( |
|
Sorry about that :( |
Yes, definitely more grammatically correct than "used as async", which makes no sense at all. |
| exports.randomBytes = exports.pseudoRandomBytes = randomBytes; | ||
| exports.randomBytes = exports.pseudoRandomBytes = function(size, callback) { | ||
| const warning = 'randomBytes used as async is deprecated. ' + | ||
| 'Use randomBytesSync instead.'; |
There was a problem hiding this comment.
wait... i'm confused now. i don't see a randomBytesSync() function - I think I added that before but it's no longer relevant. removing this line.
|
i can still add the |
|
Hmm, looking trough #5, I can't seem to find any compelling reason why async #5 (comment) might be a better reason to do a change to |
|
@silverwind won't it almost always be instantaneous? |
|
@Fishrock123 10MB of randomess take about 350ms on my Macbook, including process startup which is about 150ms, so yes - small number of bytes would be "instant". |
deprecates async usage of
.randomBytes()