-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
src, doc: remove SSLv2 constants and descriptions #5541
Conversation
Constants and doc descriptions related to SSLv2 are no longer needed. Fixes: nodejs#5529
`--enable-ssl3` flag respectively. In future versions of Node.js SSLv2 and | ||
SSLv3 will not be compiled in by default. | ||
If you wish to enable SSLv3, run node with the `--enable-ssl3` flag | ||
respectively. In future versions of Node.js SSLv3 will not be compiled in by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can drop the 'respectively' here.
Documentation changes LGTM with a nit (and good catch, I forgot about that). I left node_constants.cc unchanged on purpose although I guess removing the constants shouldn't hurt. |
The constants changes should be done on @bnoordhuis it's past midnight in Tokyo, any objection to me cherry-picking this and backing out the constants changes to get v0.10 out? |
The fix has already been made. I' m about to land it. |
Doc descriptions related to SSLv2 are no longer needed. Fixes: #5529 PR-URL: #5541 Reviewed-By: Ben Noordhuis <[email protected]>
Fixed for only doc change and Ben's comment. Landed in 6db377b. Please go a head of release v0.10. |
woo! dediation, thanks a lot @shigeki, you should probably sleep now. |
@rvagg Thanks. You too take care of yourself. |
Doc descriptions related to SSLv2 are no longer needed. Fixes: #5529 PR-URL: #5541 Reviewed-By: Ben Noordhuis <[email protected]>
added to v0.12 @ ce58c2c |
Doc descriptions related to SSLv2 are no longer needed. Fixes: nodejs/node#5529 PR-URL: nodejs/node#5541 Reviewed-By: Ben Noordhuis <[email protected]>
Pull Request check-list
Please make sure to review and check all of these items:
make -j8 test
(UNIX) orvcbuild test nosign
(Windows) pass withthis change (including linting)?
test (or a benchmark) included?
existing APIs, or introduces new ones)?
Affected core subsystem(s)
src, doc
Description of change
Constants and doc descriptions related to SSLv2 are no longer needed.
Fixes: #5529
CI of https://ci.nodejs.org/job/node-test-commit/2435/ is fine except existing lint errors and flaky tests on Win.
R= @bnoordhuis , @rvagg