-
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
src: document DoWrite() usage expectations #26339
Conversation
Clarify how it must behave for both synchronous and asynchronous completion.
@sam-github sadly an error occured when I tried to trigger a build :( |
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
re-ci: https://ci.nodejs.org/job/node-test-pull-request/21112/ It's just a header file comment, but a good build is hard to find. :-( |
re-ci: https://ci.nodejs.org/job/node-test-pull-request/21115/ windows vs2017 failed across the board, looks like remotes couldn't get connected to, but perhaps I misread the log files |
Windows rebuild: https://ci.nodejs.org/job/node-test-commit-windows-fanned/25099/ |
This should not require a full CI run as it's a doc only change. Therefore this should be good to land without making sure all platforms are green. |
Landed in 6593f77 |
Clarify how it must behave for both synchronous and asynchronous completion. PR-URL: #26339 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Clarify how it must behave for both synchronous and asynchronous completion. PR-URL: nodejs#26339 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Clarify how it must behave for both synchronous and asynchronous
completion.
@addaleax @nodejs/streams This documents my understanding of the StreamResource::DoWrite() API expectations. TLS wasn't obeying these expecations, leading to wide-spread test suite failures during TLS1.3 development.
Checklist