-
Notifications
You must be signed in to change notification settings - Fork 30k
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
doc: modernize and fix code examples in https.md #12171
Conversation
Provide relevant test file path, add missing option.
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.
LGTM, although casual readers might find the use of test/fixtures/
a bit confusing.
@cjihrig I thought the same but then I took a look at the other examples and |
Yes, I was a bit surprised too, but these test paths are already there in some other examples, so I've just tried to conform. It seems it is easier to find these files in the code base than to make them somehow for quick testing. |
Landed in 5059363 |
* Replace `var` by `const`. * Comment out ellipses. * Update code example (provide relevant file path, add missing option). PR-URL: #12171 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* Replace `var` by `const`. * Comment out ellipses. * Update code example (provide relevant file path, add missing option). PR-URL: nodejs#12171 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* Replace `var` by `const`. * Comment out ellipses. * Update code example (provide relevant file path, add missing option). PR-URL: #12171 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
I've backported to v6.x assuming the changes are relevant. Let me know if this should be backed out |
* Replace `var` by `const`. * Comment out ellipses. * Update code example (provide relevant file path, add missing option). PR-URL: #12171 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* Replace `var` by `const`. * Comment out ellipses. * Update code example (provide relevant file path, add missing option). PR-URL: nodejs/node#12171 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Checklist
Affected core subsystem(s)
doc, https
var
byconst