-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: replace server.close()
which don't exist in code snippets in tls.md
#23239
Conversation
doc/api/tls.md
Outdated
@@ -944,7 +944,7 @@ socket.on('data', (data) => { | |||
console.log(data); | |||
}); | |||
socket.on('end', () => { | |||
server.close(); | |||
console.log('client ends') |
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.
Linter complains about missing semicolons.
Not opposed to this change, but I think we can do even better. The comment above the code samples says it implements an "echo server". The client is connecting on port 8000 to localhost but that is not explained. The code samples might be improved by adding the server code (listening on loclahost port 8000) and leaving |
(I guess since this is documenting |
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. I left some comments about other stuff that can be done to improve this particular bit of sample code, and that can certainly happen in this PR, but it can also happen in a subsequent PR or not at all.
I agree, this sentence |
Thanks for your advice. Let me improve it a bit. |
This sentence seems to be changed by accident in 1b6a468. |
Landed in 0544816 |
Replace `server.close()` which don't exist in code snippets. PR-URL: #23239 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Replace `server.close()` which don't exist in code snippets. PR-URL: #23239 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Replace `server.close()` which don't exist in code snippets. PR-URL: #23239 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Replace `server.close()` which don't exist in code snippets. PR-URL: #23239 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Replace
server.close()
as theserver
don't exist in the code snippets intls.md
.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes