fs: fix cb/sync writev empty array behavior #41932
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ronag
reviewed
Feb 11, 2022
This comment has been minimized.
This comment has been minimized.
aduh95
approved these changes
Feb 11, 2022
|
CI: https://ci.nodejs.org/job/node-test-pull-request/42494/ (aborted |
19 tasks
ronag
approved these changes
Feb 12, 2022
jasnell
approved these changes
Feb 12, 2022
19 tasks
18 tasks
|
Not sure what's up with CI I'll try a new run |
Commit Queue failed- Loading data for nodejs/node/pull/41932 ✔ Done loading data for nodejs/node/pull/41932 ----------------------------------- PR info ------------------------------------ Title fs: fix cb/sync writev empty array behavior (#41932) Author Benjamin Gruenbaum (@benjamingr) Branch benjamingr:fix-writeev-sync-cb -> nodejs:master Labels fs, needs-ci Commits 2 - fs: fix cb/sync writev empty array behavior - fixup! ensure cb behavior Committers 1 - Benjamin Gruenbaum PR-URL: https://github.com/nodejs/node/pull/41932 Refs: https://github.com/nodejs/node/issues/41910 Reviewed-By: Antoine du Hamel Reviewed-By: Robert Nagy Reviewed-By: James M Snell ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/41932 Refs: https://github.com/nodejs/node/issues/41910 Reviewed-By: Antoine du Hamel Reviewed-By: Robert Nagy Reviewed-By: James M Snell -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 11 Feb 2022 10:33:55 GMT ✔ Approvals: 3 ✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/41932#pullrequestreview-880033400 ✔ - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/41932#pullrequestreview-880871265 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/41932#pullrequestreview-880871990 ✖ GitHub CI is still running ℹ Last Full PR CI on 2022-02-14T12:05:58Z: https://ci.nodejs.org/job/node-test-pull-request/42549/ - Querying data for job/node-test-pull-request/42549/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/1842083455 |
|
Not sure why pending is showing |
|
@nodejs/build this is showing "passed" on Jenkins but GitHub is not updated. I'll land manually but pinging in case you want to investigate. |
|
Landed in aff8b87 |
20 tasks
33 tasks
28 tasks
bengl
added a commit
to bengl/node
that referenced
this issue
Feb 21, 2022
PR-URL: nodejs#41932 Refs: nodejs#41910 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com>
bengl
added a commit
to bengl/node
that referenced
this issue
Feb 21, 2022
PR-URL: nodejs#41932 Refs: nodejs#41910 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
This change aligns the cb/sync behavior of writev with the promises one at #41919
I suspect the old behavior was a bug since it wasn't tested or documented so I am treating this as a bug fix rather than a behavior change but I am open to changing that if people feel differently.
Refs: #41910
cc @aduh95 @ronag @JungMinu
The text was updated successfully, but these errors were encountered: