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.

Sign up for GitHub

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

src: prefer false instead of bool zero #20218

Closed
wants to merge 2 commits into from
Closed

src: prefer false instead of bool zero #20218

wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Apr 23, 2018

This commit updates node_crypto.cc VerifySpkac function to use false
instead of 0 for its return bool value i.

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

@danbev
src: prefer false instead of bool zero
c77c43d 
This commit updates node_crypto.cc VerifySpkac function to use false
instead of 0 for its return bool value i.
@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 Apr 23, 2018
addaleax
addaleax approved these changes Apr 23, 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.

While you’re at it … it’s not really obvious what i might mean in this context, tbh.

@danbev
Copy link
Contributor Author

danbev commented Apr 23, 2018

While you’re at it … it’s not really obvious what i might mean in this context, tbh.

Yeah, I was thinking the same thing. i is used in few other functions as well. I'll go through and update them too.

@danbev
src: rename return var in VerifySpkac functions
7274a5a 
This commit renames the verification result variable, that is currently
named i, to verify_result.
@addaleax addaleax added fast-track PRs that do not need to wait for 48 hours to land. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Apr 23, 2018
@addaleax
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-commit/17944/

Please 👍 this comment if it’s okay to fast-track this.

@addaleax
Copy link
Member

Landed in e767aa1...c07c73c

@addaleax addaleax closed this Apr 23, 2018
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 23, 2018
addaleax pushed a commit that referenced this pull request Apr 23, 2018
@danbev @addaleax
src: prefer false instead of bool zero
e3b0fb0 
This commit updates node_crypto.cc VerifySpkac function to use false
instead of 0 for its return bool value i.

PR-URL: #20218
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Apr 23, 2018
@danbev @addaleax
src: rename return var in VerifySpkac functions
c07c73c 
This commit renames the verification result variable, that is currently
named i, to verify_result.

PR-URL: #20218
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 4, 2018
@danbev @MylesBorins
src: prefer false instead of bool zero
a4dae6c 
This commit updates node_crypto.cc VerifySpkac function to use false
instead of 0 for its return bool value i.

PR-URL: #20218
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 4, 2018
@danbev @MylesBorins
src: rename return var in VerifySpkac functions
bd2e521 
This commit renames the verification result variable, that is currently
named i, to verify_result.

PR-URL: #20218
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request May 8, 2018
Merged
@danbev danbev deleted the crypto_prefer_false_to_zero branch August 20, 2019 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

@JungMinu JungMinu JungMinu approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@cjihrig cjihrig cjihrig approved these changes

@targos targos targos approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. fast-track PRs that do not need to wait for 48 hours to land.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@danbev @addaleax @jasnell @targos @cjihrig @joyeecheung @JungMinu @richardlau @nodejs-github-bot