Skip to content

crypto: simplify GetPublicOrPrivateKeyFromJs - #26454

Closed
tniessen wants to merge 1 commit into
nodejs:masterfrom
tniessen:crypto-simplify-getpublicorprivatekeyfromjs
Closed

crypto: simplify GetPublicOrPrivateKeyFromJs#26454
tniessen wants to merge 1 commit into
nodejs:masterfrom
tniessen:crypto-simplify-getpublicorprivatekeyfromjs

Conversation

@tniessen

@tniessen tniessen commented Mar 5, 2019

Copy link
Copy Markdown
Member

The last argument is now always set to true due to #26278. This change removes said argument and its only usage in an assertion.

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

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. labels Mar 5, 2019

@bnoordhuis bnoordhuis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I felt it should have been an enum in the first place. :-)

@BridgeAR

BridgeAR commented Mar 5, 2019

Copy link
Copy Markdown
Member

@BridgeAR BridgeAR added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Mar 5, 2019
@shisama

shisama commented Mar 6, 2019

Copy link
Copy Markdown
Contributor

@tniessen

tniessen commented Mar 6, 2019

Copy link
Copy Markdown
Member Author

@tniessen

tniessen commented Mar 7, 2019

Copy link
Copy Markdown
Member Author

Thanks for reviewing, everyone! Landed in 3e4e518.

@tniessen tniessen closed this Mar 7, 2019
tniessen added a commit that referenced this pull request Mar 7, 2019
PR-URL: #26454
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Mar 12, 2019
PR-URL: nodejs#26454
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
BridgeAR pushed a commit that referenced this pull request Mar 14, 2019
PR-URL: #26454
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
@tniessen tniessen removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants