-
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
doc: improve net docs #32811
doc: improve net docs #32811
Conversation
@mcollina What is your opinion on rather than duplicating docs (which over time might end up being inconsistent or outdated) we just refer to the stream docs? e.g.
|
Refer back to streams docs for further and more accurate description of behavior details. Refs: nodejs#31916
Co-Authored-By: mscdex <[email protected]>
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
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
doc/api/net.md
Outdated
Ensures that no more I/O activity happens on this socket. Only necessary in | ||
case of errors (parse error or so). | ||
Ensures that no more I/O activity happens on this socket. This destroys | ||
the stream but does not explicitly cause a TCP stream reset (RST). |
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.
@lpinca: please note updated version, better?
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.
I would honestly not add those details that I think are confusing for most users. "Destroys the stream and closes the connection" is enough. Interested users can dig by themselves to see what happens under the hood.
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.
Changed to your suggestion.
Co-Authored-By: mscdex <[email protected]>
Refer back to streams docs for further and more accurate description of behavior details. Refs: #31916 PR-URL: #32811 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>
Landed in 9534458 |
Refer back to streams docs for further and more accurate description of behavior details. Refs: #31916 PR-URL: #32811 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>
Refer back to streams docs for further and more accurate description of behavior details. Refs: nodejs#31916 PR-URL: nodejs#32811 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>
Refer back to streams docs for further and more accurate description of behavior details. Refs: #31916 PR-URL: #32811 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>
Refer back to streams docs for further and more accurate description of behavior details. Refs: #31916 PR-URL: #32811 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>
Clarify destroy behaviour and refer back to streams docs for further and more accurate
description of behavior details.
Refs: #31916
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes