Skip to content

crypto: remove redundant validateUint32 argument #30579

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Conversation

tniessen
Copy link
Member

The third parameter should be a boolean indicating whether the number must be positive. Passing zero works, but is unnecessary, misleading and inconsistent with other uses of the same function.

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

The third parameter should be a boolean indicating whether the number
must be positive. Passing zero works, but is unnecessary, misleading
and inconsistent with other uses of the same function.
@nodejs-github-bot nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Nov 21, 2019
@tniessen
Copy link
Member Author

Please add 👍 here to approve fast-tracking.

@tniessen tniessen added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 21, 2019
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@nodejs-github-bot
Copy link
Collaborator

tniessen added a commit that referenced this pull request Nov 22, 2019
The third parameter should be a boolean indicating whether the number
must be positive. Passing zero works, but is unnecessary, misleading
and inconsistent with other uses of the same function.

PR-URL: #30579
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@tniessen
Copy link
Member Author

Landed in 64ee64d, thanks for reviewing.

@tniessen tniessen closed this Nov 22, 2019
addaleax pushed a commit that referenced this pull request Nov 30, 2019
The third parameter should be a boolean indicating whether the number
must be positive. Passing zero works, but is unnecessary, misleading
and inconsistent with other uses of the same function.

PR-URL: #30579
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Dec 1, 2019
The third parameter should be a boolean indicating whether the number
must be positive. Passing zero works, but is unnecessary, misleading
and inconsistent with other uses of the same function.

PR-URL: #30579
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Dec 3, 2019
@BethGriggs BethGriggs mentioned this pull request Dec 9, 2019
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
The third parameter should be a boolean indicating whether the number
must be positive. Passing zero works, but is unnecessary, misleading
and inconsistent with other uses of the same function.

PR-URL: #30579
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Dec 23, 2019
@tniessen tniessen deleted the crypto-remove-redundant-validateuint32-arg branch January 23, 2020 03:48
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

5 participants