Skip to content

Conversation

tniessen
Copy link
Member

This is a slight reduction in SLOC by deduplicating public key parsing in the crypto module as suggested in a comment in lines 3715 to 3717. There should be no functional difference.

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

@nodejs-github-bot
Copy link
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 Aug 27, 2018
@tniessen
Copy link
Member Author

cc @nodejs/crypto

@@ -3648,6 +3648,45 @@ void Sign::SignFinal(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(rc);
}

typedef enum {
Copy link
Member

Choose a reason for hiding this comment

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

No need to typedef, enum ParsePublicKeyResult works too :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, Anna :)

@tniessen
Copy link
Member Author

tniessen commented Aug 29, 2018

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

The last two CI builds passed according to Jenkins, but GH is still displaying pending / failed. cc @nodejs/build

@addaleax
Copy link
Member

Landed in 83f319c

@addaleax addaleax closed this Aug 31, 2018
addaleax pushed a commit that referenced this pull request Aug 31, 2018
PR-URL: #22553
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Aug 31, 2018
PR-URL: #22553
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@tniessen tniessen mentioned this pull request Sep 2, 2018
4 tasks
targos pushed a commit that referenced this pull request Sep 3, 2018
PR-URL: #22553
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Sep 6, 2018
PR-URL: #22553
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@tniessen tniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 12, 2018
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.

3 participants