-
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
docs: fix description of _writev() #28690
Conversation
The documentation gave the impression that when the _writev method is used, it is used instead of _write. However, when _writev is used, _write loads the first chunk of data, then _writev loads all the remaining bufferred chunks in the write queue. The docs have been changed to reflect this behavior. Fixes: nodejs#28408
multiple chunks of data at once. If implemented, the method will be called with | ||
all chunks of data currently buffered in the write queue. | ||
multiple chunks of data at once. If implemented, the first chunk will be written with | ||
`_write()`. When the initial write is complete, buffered chunks in the write queue, if available, |
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'm pretty sure our linter will complain that this line is longer than 80 chars and has trailing white space. At least I hope it will flag those things. Run make lint-md
(or vcbuild lint-md
if on Windows) to run the documentation linter locally.
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.
Also: Welcome @tyof45 and thanks for the pull request!
multiple chunks of data at once. If implemented, the method will be called with | ||
all chunks of data currently buffered in the write queue. | ||
multiple chunks of data at once. If implemented, the first chunk will be written with | ||
`_write()`. When the initial write is complete, buffered chunks in the write queue, if available, |
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 think it’s not just the first chunk, but rather, _write()
is always used when there is only one chunk in the queue?
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.
When the queue is empty.
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.
Isn't current docs correct in that case @addaleax @lpinca ? Though still bit ambiguous. Does below seems more correct -
if implemented, only the
writable._writev()
method will be called with all chunks of data currently buffered in the write queue. In case there is only a single data chunk to be processedwritable._write()
is called instead.
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.
@erabhimanyu I think your suggestion is still confusing. In particular this sentence
In case there is only a single data chunk to be processed
writable._write()
is called instead
If there is buffered data even a single chunk, writable._writev()
will be used.
Ping @tyof45 this needs a rebase. |
the exact context of invocation of _writev API is not well known also, the choice between _write and _writev is not well known. add a description to make it explicit. Fixes: nodejs#28408 Refs: nodejs#28690 Co-authored-by: Parker Bjur <bjur.parker45@gmail.com>
The exact context of invocation of _writev API is not well known. Also, the choice between _write and _writev is not well known. Add a description to make it explicit. Fixes: #28408 Refs: #28690 Co-authored-by: Parker Bjur <bjur.parker45@gmail.com> PR-URL: #31356 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
The exact context of invocation of _writev API is not well known. Also, the choice between _write and _writev is not well known. Add a description to make it explicit. Fixes: #28408 Refs: #28690 Co-authored-by: Parker Bjur <bjur.parker45@gmail.com> PR-URL: #31356 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
The exact context of invocation of _writev API is not well known. Also, the choice between _write and _writev is not well known. Add a description to make it explicit. Fixes: #28408 Refs: #28690 Co-authored-by: Parker Bjur <bjur.parker45@gmail.com> PR-URL: #31356 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
The exact context of invocation of _writev API is not well known. Also, the choice between _write and _writev is not well known. Add a description to make it explicit. Fixes: #28408 Refs: #28690 Co-authored-by: Parker Bjur <bjur.parker45@gmail.com> PR-URL: #31356 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
This can be closed as the intention is fulfilled in #31356. |
The documentation gave the impression that when the _writev method is
used, it is used instead of _write. However, when _writev is used,
_write loads the first chunk of data, then _writev loads all the
remaining bufferred chunks in the write queue. The docs have been
changed to reflect this behavior.
Fixes: #28408
Checklist