crypto: use CHECK instead in getSSLCiphers #16453
Conversation
jasnell
added
crypto
nodejs-github-bot
added
c++
src/node_crypto.cc
Outdated
There was a problem hiding this comment.
SSL_new - although the comments are a bit superfluous, IMO.
src/node_crypto.cc
Outdated
There was a problem hiding this comment.
This change seems unrelated.
There was a problem hiding this comment.
Unrelated, yes, but while I was in here I figured I'd clean up a bit.
There was a problem hiding this comment.
A commit should not contain unrelated cleanup. It should do what it says on the tin, no more and no less.
There was a problem hiding this comment.
Will update the commit message to include the additional change
There was a problem hiding this comment.
Ah, that's what I get for using pithy one-liners. What I mean is, cleanup is fine but do it in a separate commit. Future code archeologists will thank you for it.
Having said that, this specific cleanup doesn't buy much. It's neither shorter nor faster.
|
CI failures are unrelated. |
jasnell
force-pushed
the
crypto-getsslciphers-refactor
branch
from
a961d46 to
dba4c22
Compare
|
Ping @nodejs/tsc |
The previous throws should never happen, and if they do, they signal a larger issue in core. Make these checks rather than throws.
jasnell
force-pushed
the
crypto-getsslciphers-refactor
branch
from
42fc0d7 to
1ab7c6b
Compare
|
@bnoordhuis ... updated to remove the errant change. PTAL |
|
New CI, just to be safe: https://ci.nodejs.org/job/node-test-pull-request/11003/ |
The previous throws should never happen, and if they do, they signal a larger issue in core. Make these checks rather than throws. PR-URL: #16453 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
|
Landed in df8c6c3 |
The previous throws should never happen, and if they do, they signal a larger issue in core. Make these checks rather than throws. PR-URL: nodejs/node#16453 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
The previous throws should never happen, and if they do, they signal a larger issue in core. Make these checks rather than throws. PR-URL: nodejs/node#16453 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
5 participants
The previous throws should never happen, and if they do, they signal a larger issue in core. Make these checks rather than throws
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
crypto