crypto: move _scrypt call out of handleError funct by danbev · Pull Request #28318 · nodejs/node

danbev · 2019-06-20T06:21:56Z

This commit moves the _scrypt function call out of the handleError
function, which now only takes in an error object as its parameter.

The motivation for this is to hopefully improve readability as it was
not clear to me the first time I stepped through the code where the
actual call to _scrypt was.

Checklist

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

This commit moves the _scrypt function call out of the handleError function, which now only takes in an error object as its parameter. The motivation for this is to hopefully improve readability as it was not clear to me the first time I stepped through the code where the actual call to _scrypt was.

nodejs-github-bot · 2019-06-20T06:22:49Z

CI: https://ci.nodejs.org/job/node-test-pull-request/24000/

bnoordhuis

If you're going to make this change, you should also make it to pbkdf2.js, random.js and (possibly) keygen.js, otherwise you're introducing style inconsistencies between the files in lib/internal/crypto.

lib/internal/crypto/scrypt.js

danbev · 2019-06-20T10:00:34Z

f you're going to make this change, you should also make it to pbkdf2.js, random.js and (possibly) keygen.js

Ah I see, let me take a look and update them too. Thanks

This commit moves the _randomBytes function call out of the handleError function, which now it takes in an error and a buf object as its parameters.

This commit moves the _pbkdf2 function call out of the handleError function, which now only takes in an error and a digest object as its parameters.

This commit moves the _impl function call out of the handleError function, which now only takes in an object as its parameter.

nodejs-github-bot · 2019-06-20T12:03:37Z

CI: https://ci.nodejs.org/job/node-test-pull-request/24004/

bnoordhuis

LGTM although it drops the D in DRY.

nodejs-github-bot · 2019-06-21T09:40:51Z

CI: https://ci.nodejs.org/job/node-test-pull-request/24033/

nodejs-github-bot · 2019-06-21T13:40:18Z

CI: https://ci.nodejs.org/job/node-test-pull-request/24038/

danbev · 2019-06-24T03:44:24Z

Landed in 882e8fe, 556b38a, 7edd5f3, and c6f968d.

This commit moves the _scrypt function call out of the handleError function, which now only takes in an error object as its parameter. The motivation for this is to hopefully improve readability as it was not clear to me the first time I stepped through the code where the actual call to _scrypt was. PR-URL: #28318 Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>

This commit moves the _randomBytes function call out of the handleError function, which now it takes in an error and a buf object as its parameters. PR-URL: #28318 Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>

This commit moves the _pbkdf2 function call out of the handleError function, which now only takes in an error and a digest object as its parameters. PR-URL: #28318 Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>

This commit moves the _impl function call out of the handleError function, which now only takes in an object as its parameter. PR-URL: #28318 Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>

This commit moves the _scrypt function call out of the handleError function, which now only takes in an error object as its parameter. The motivation for this is to hopefully improve readability as it was not clear to me the first time I stepped through the code where the actual call to _scrypt was. PR-URL: #28318 Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>

This commit moves the _randomBytes function call out of the handleError function, which now it takes in an error and a buf object as its parameters. PR-URL: #28318 Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>

This commit moves the _pbkdf2 function call out of the handleError function, which now only takes in an error and a digest object as its parameters. PR-URL: #28318 Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>

This commit moves the _impl function call out of the handleError function, which now only takes in an object as its parameter. PR-URL: #28318 Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Jun 20, 2019

ZYSzys approved these changes Jun 20, 2019

View reviewed changes

benjamingr approved these changes Jun 20, 2019

View reviewed changes

bnoordhuis requested changes Jun 20, 2019

View reviewed changes

lib/internal/crypto/scrypt.js Outdated Show resolved Hide resolved

fixup! crypto: move _scrypt call out of handleError funct

ec54953

danbev added 3 commits June 20, 2019 13:27

crypto: move _randomBytes call out of handleError funct

3b3908d

This commit moves the _randomBytes function call out of the handleError function, which now it takes in an error and a buf object as its parameters.

crypto: move _pbkdf2 call out of handleError funct

d96eff6

This commit moves the _pbkdf2 function call out of the handleError function, which now only takes in an error and a digest object as its parameters.

crypto: move _impl call out of handleError funct

247df3c

This commit moves the _impl function call out of the handleError function, which now only takes in an object as its parameter.

addaleax approved these changes Jun 20, 2019

View reviewed changes

bnoordhuis approved these changes Jun 21, 2019

View reviewed changes

Trott approved these changes Jun 21, 2019

View reviewed changes

Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 21, 2019

tniessen approved these changes Jun 21, 2019

View reviewed changes

jasnell approved these changes Jun 23, 2019

View reviewed changes

danbev closed this Jun 24, 2019

danbev deleted the scrypt_handleError branch June 24, 2019 03:44

targos mentioned this pull request Jul 2, 2019

Release v12.6.0 proposal #28508

Merged

This was referenced Jul 3, 2019

2019-07-03 Version 12.6.0 (Current) @targos beevelop/docker-nodejs#288

Closed

2019-07-03 Version 12.6.0 (Current) @targos beevelop/docker-nodejs#289

Closed

2019-07-03 Version 12.6.0 (Current) @targos beevelop/docker-nodejs#290

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

crypto: move _scrypt call out of handleError funct#28318

crypto: move _scrypt call out of handleError funct#28318
danbev wants to merge 5 commits intonodejs:masterfrom
danbev:scrypt_handleError

danbev commented Jun 20, 2019

Uh oh!

nodejs-github-bot commented Jun 20, 2019

Uh oh!

bnoordhuis left a comment

Uh oh!

Uh oh!

danbev commented Jun 20, 2019

Uh oh!

nodejs-github-bot commented Jun 20, 2019

Uh oh!

bnoordhuis left a comment

Uh oh!

nodejs-github-bot commented Jun 21, 2019

Uh oh!

nodejs-github-bot commented Jun 21, 2019

Uh oh!

danbev commented Jun 24, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Uh oh!

Conversation

danbev commented Jun 20, 2019

Checklist

Uh oh!

nodejs-github-bot commented Jun 20, 2019

Uh oh!

bnoordhuis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danbev commented Jun 20, 2019

Uh oh!

nodejs-github-bot commented Jun 20, 2019

Uh oh!

bnoordhuis left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jun 21, 2019

Uh oh!

nodejs-github-bot commented Jun 21, 2019

Uh oh!

danbev commented Jun 24, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants