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: add SET_INTEGER_CONSTANT macro #23687

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Oct 16, 2018

This commit introduces a SET_INTEGER_CONSTANT macro to reduce some code
duplication in SecureContext::Initialize.

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

This commit introduces a SET_INTEGER_CONSANT macro to reduce some code
duplication in SecureContext::Initialize.
@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 Oct 16, 2018
@danbev
Copy link
Contributor Author

danbev commented Oct 16, 2018

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Could also be an inline/lambda function this way :)

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 16, 2018
@aduh95
Copy link
Contributor

aduh95 commented Oct 16, 2018

Note: typo in the title of the PR as well as in the commit message CONSANT -> CONSTANTS

@danbev danbev changed the title crypto: add SET_INTEGER_CONSANT macro crypto: add SET_INTEGER_CONSTANT macro Oct 17, 2018
@danbev
Copy link
Contributor Author

danbev commented Oct 17, 2018

Note: typo in the title of the PR as well as in the commit message CONSANT -> CONSTANTS

I've updated the commit message now. Thanks!

@danbev
Copy link
Contributor Author

danbev commented Oct 17, 2018

Re-run of failing node-test-commit

@danbev
Copy link
Contributor Author

danbev commented Oct 18, 2018

Could also be an inline/lambda function this way :)

That was my first choice but I ran into an issue with the string literal and passing it to the lambda operator function. I'll take another look at this later today and see if I can figure out some way around it .

@danbev
Copy link
Contributor Author

danbev commented Oct 19, 2018

Re-run of failing node-test-commit-linux-containered

@danbev
Copy link
Contributor Author

danbev commented Oct 24, 2018

Landed in 30219bf.

@danbev danbev closed this Oct 24, 2018
@danbev danbev deleted the crypto_set_integer_macro branch October 24, 2018 03:48
danbev added a commit that referenced this pull request Oct 24, 2018
This commit introduces a SET_INTEGER_CONSANT macro to reduce some code
duplication in SecureContext::Initialize.

PR-URL: #23687
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
targos pushed a commit that referenced this pull request Oct 24, 2018
This commit introduces a SET_INTEGER_CONSANT macro to reduce some code
duplication in SecureContext::Initialize.

PR-URL: #23687
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
This commit introduces a SET_INTEGER_CONSANT macro to reduce some code
duplication in SecureContext::Initialize.

PR-URL: #23687
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
This commit introduces a SET_INTEGER_CONSANT macro to reduce some code
duplication in SecureContext::Initialize.

PR-URL: #23687
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@codebytere codebytere mentioned this pull request Nov 27, 2018
rvagg pushed a commit that referenced this pull request Nov 28, 2018
This commit introduces a SET_INTEGER_CONSANT macro to reduce some code
duplication in SecureContext::Initialize.

PR-URL: #23687
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
This commit introduces a SET_INTEGER_CONSANT macro to reduce some code
duplication in SecureContext::Initialize.

PR-URL: #23687
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
This commit introduces a SET_INTEGER_CONSANT macro to reduce some code
duplication in SecureContext::Initialize.

PR-URL: #23687
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@codebytere codebytere mentioned this pull request Nov 29, 2018
@BethGriggs BethGriggs mentioned this pull request Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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.

9 participants