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

Jump to bottom

crypto: add SET_INTEGER_CONSTANT macro #23687

Closed
wants to merge 1 commit into from
Closed

crypto: add SET_INTEGER_CONSTANT macro #23687

wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Oct 16, 2018 •
edited

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

@danbev
crypto: add SET_INTEGER_CONSANT macro
120c51e 
This commit introduces a SET_INTEGER_CONSANT macro to reduce some code
duplication in SecureContext::Initialize.
@nodejs-github-bot
Copy link
Collaborator

@danbev build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1259/pipeline

@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

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

addaleax
addaleax approved these changes Oct 16, 2018
View reviewed changes
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 •
edited

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
@danbev
crypto: add SET_INTEGER_CONSANT macro
30219bf 
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
@danbev @targos
crypto: add SET_INTEGER_CONSANT macro
141aec9 
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 targos mentioned this pull request Oct 27, 2018
Merged
This was referenced Nov 2, 2018
Closed
Closed
Closed
Closed
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
@danbev @MylesBorins
crypto: add SET_INTEGER_CONSANT macro
d62d666 
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
@danbev @MylesBorins
crypto: add SET_INTEGER_CONSANT macro
b55b5a7 
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
Closed
rvagg pushed a commit that referenced this pull request Nov 28, 2018
@danbev @rvagg
crypto: add SET_INTEGER_CONSANT macro
4996056 
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
@danbev @rvagg
crypto: add SET_INTEGER_CONSANT macro
5dc2fa2 
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
@danbev @MylesBorins
crypto: add SET_INTEGER_CONSANT macro
a97e79c 
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
Merged
@BethGriggs BethGriggs mentioned this pull request Dec 4, 2018
Merged
This was referenced Dec 18, 2018
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@thefourtheye thefourtheye thefourtheye approved these changes

@addaleax addaleax addaleax approved these changes

@tniessen tniessen tniessen approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@danbev @nodejs-github-bot @aduh95 @jasnell @thefourtheye @addaleax @tniessen @MylesBorins @BridgeAR