Skip to content
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

fs: add flush option to createWriteStream() #50093

Merged
merged 4 commits into from
Oct 11, 2023

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Oct 8, 2023

This commit adds a 'flush' option to the createWriteStream() family of functions.

Refs: #49886

@cjihrig cjihrig requested a review from mcollina October 8, 2023 22:01
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Oct 8, 2023
This commit adds a 'flush' option to the createWriteStream()
family of functions.

Refs: nodejs#49886
lib/internal/fs/streams.js Outdated Show resolved Hide resolved
lib/internal/fs/streams.js Show resolved Hide resolved
lib/internal/fs/streams.js Outdated Show resolved Hide resolved
lib/internal/fs/streams.js Outdated Show resolved Hide resolved
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina added commit-queue Add this label to land a pull request using GitHub Actions. request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 10, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 10, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@mcollina mcollina added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Oct 11, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 11, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
node:internal/modules/cjs/loader:1077
  const err = new Error(message);
              ^

Error: Cannot find module 'call-bind'
Require stack:

  • /opt/hostedtoolcache/node/18.18.0/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1077:15)
    at Module._load (node:internal/modules/cjs/loader:922:27)
    at Module.require (node:internal/modules/cjs/loader:1143:19)
    at require (node:internal/modules/cjs/helpers:119:18)
    at Object. (/opt/hostedtoolcache/node/18.18.0/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js:4:16)
    at Module._compile (node:internal/modules/cjs/loader:1256:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
    at Module.load (node:internal/modules/cjs/loader:1119:32)
    at Module._load (node:internal/modules/cjs/loader:960:12)
    at ModuleWrap. (node:internal/modules/esm/translators:169:29) {
    code: 'MODULE_NOT_FOUND',
    requireStack: [
    '/opt/hostedtoolcache/node/18.18.0/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js'
    ]
    }

Node.js v18.18.0

https://github.com/nodejs/node/actions/runs/6480389717

@mcollina mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 11, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 11, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
node:internal/modules/cjs/loader:1077
  const err = new Error(message);
              ^

Error: Cannot find module 'call-bind'
Require stack:

  • /opt/hostedtoolcache/node/18.18.0/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1077:15)
    at Module._load (node:internal/modules/cjs/loader:922:27)
    at Module.require (node:internal/modules/cjs/loader:1143:19)
    at require (node:internal/modules/cjs/helpers:119:18)
    at Object. (/opt/hostedtoolcache/node/18.18.0/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js:4:16)
    at Module._compile (node:internal/modules/cjs/loader:1256:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
    at Module.load (node:internal/modules/cjs/loader:1119:32)
    at Module._load (node:internal/modules/cjs/loader:960:12)
    at ModuleWrap. (node:internal/modules/esm/translators:169:29) {
    code: 'MODULE_NOT_FOUND',
    requireStack: [
    '/opt/hostedtoolcache/node/18.18.0/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js'
    ]
    }

Node.js v18.18.0

https://github.com/nodejs/node/actions/runs/6480615480

@mcollina
Copy link
Member

@nodejs/build could somebody take a look while the commit-queue automation is failing?

@richardlau
Copy link
Member

@nodejs/build could somebody take a look while the commit-queue automation is failing?

This looks like ljharb/through#1 cc @ljharb .

@ljharb
Copy link
Member

ljharb commented Oct 11, 2023

Fixed; published v2.3.11 of @ljharb/through.

@richardlau richardlau added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 11, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 11, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/50093
✔  Done loading data for nodejs/node/pull/50093
----------------------------------- PR info ------------------------------------
Title      fs: add flush option to createWriteStream() (#50093)
Author     Colin Ihrig  (@cjihrig)
Branch     cjihrig:stream-flush -> nodejs:main
Labels     fs, author ready
Commits    4
 - fs: add flush option to createWriteStream()
 - nits
 - nit
 - linter
Committers 1
 - cjihrig 
PR-URL: https://github.com/nodejs/node/pull/50093
Refs: https://github.com/nodejs/node/issues/49886
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Matteo Collina 
Reviewed-By: Marco Ippolito 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50093
Refs: https://github.com/nodejs/node/issues/49886
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Matteo Collina 
Reviewed-By: Marco Ippolito 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 08 Oct 2023 22:01:09 GMT
   ✔  Approvals: 3
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/50093#pullrequestreview-1663616804
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/50093#pullrequestreview-1663977809
   ✔  - Marco Ippolito (@marco-ippolito): https://github.com/nodejs/node/pull/50093#pullrequestreview-1667059864
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-10-11T08:12:31Z: https://ci.nodejs.org/job/node-test-pull-request/54694/
- Querying data for job/node-test-pull-request/54694/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
   ed49722a8a..f23a9353ae  main       -> origin/main
✔  origin/main is now up-to-date
main is out of sync with origin/main. Mismatched commits:
 - 503f0f348b deps: update corepack to 0.21.0
 - f23a9353ae deps: update corepack to 0.21.0
--------------------------------------------------------------------------------
HEAD is now at f23a9353ae deps: update corepack to 0.21.0
   ✔  Reset to origin/main
- Downloading patch for 50093
From https://github.com/nodejs/node
 * branch                  refs/pull/50093/merge -> FETCH_HEAD
✔  Fetched commits as f23a9353ae83..ff247dfad516
--------------------------------------------------------------------------------
Auto-merging doc/api/fs.md
Auto-merging lib/fs.js
[main 0f635c4b76] fs: add flush option to createWriteStream()
 Author: cjihrig 
 Date: Sun Oct 8 17:58:50 2023 -0400
 5 files changed, 116 insertions(+), 4 deletions(-)
 create mode 100644 test/parallel/test-fs-write-stream-flush.js
[main c547629ef9] nits
 Author: cjihrig 
 Date: Sun Oct 8 18:48:51 2023 -0400
 1 file changed, 6 insertions(+), 3 deletions(-)
[main b3040a1c1d] nit
 Author: cjihrig 
 Date: Sun Oct 8 19:03:00 2023 -0400
 1 file changed, 1 deletion(-)
[main 8fc285dc93] linter
 Author: cjihrig 
 Date: Sun Oct 8 19:04:24 2023 -0400
 1 file changed, 1 insertion(+), 1 deletion(-)
   ✔  Patches applied
There are 4 commits in the PR. Attempting autorebase.
Rebasing (2/8)

Executing: git node land --amend --yes
⚠ Found Refs: #49886, skipping..
--------------------------------- New Message ----------------------------------
fs: add flush option to createWriteStream()

This commit adds a 'flush' option to the createWriteStream()
family of functions.

Refs: #49886
PR-URL: #50093
Reviewed-By: Yagiz Nizipli [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Marco Ippolito [email protected]

[detached HEAD 4e5e812a12] fs: add flush option to createWriteStream()
Author: cjihrig [email protected]
Date: Sun Oct 8 17:58:50 2023 -0400
5 files changed, 116 insertions(+), 4 deletions(-)
create mode 100644 test/parallel/test-fs-write-stream-flush.js
Rebasing (3/8)
Rebasing (4/8)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
nits

PR-URL: #50093
Refs: #49886
Reviewed-By: Yagiz Nizipli [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Marco Ippolito [email protected]

[detached HEAD 88a2b174b8] nits
Author: cjihrig [email protected]
Date: Sun Oct 8 18:48:51 2023 -0400
1 file changed, 6 insertions(+), 3 deletions(-)
Rebasing (5/8)
Rebasing (6/8)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
nit

PR-URL: #50093
Refs: #49886
Reviewed-By: Yagiz Nizipli [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Marco Ippolito [email protected]

[detached HEAD 33fec10967] nit
Author: cjihrig [email protected]
Date: Sun Oct 8 19:03:00 2023 -0400
1 file changed, 1 deletion(-)
Rebasing (7/8)
Rebasing (8/8)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
linter

PR-URL: #50093
Refs: #49886
Reviewed-By: Yagiz Nizipli [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Marco Ippolito [email protected]

[detached HEAD 71eec253ba] linter
Author: cjihrig [email protected]
Date: Sun Oct 8 19:04:24 2023 -0400
1 file changed, 1 insertion(+), 1 deletion(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/6485039679

@richardlau richardlau added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 11, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 11, 2023
@nodejs-github-bot nodejs-github-bot merged commit 8a49735 into nodejs:main Oct 11, 2023
78 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 8a49735

@cjihrig cjihrig deleted the stream-flush branch October 11, 2023 19:38
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
This commit adds a 'flush' option to the createWriteStream()
family of functions.

Refs: nodejs#49886
PR-URL: nodejs#50093
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
targos pushed a commit that referenced this pull request Nov 11, 2023
This commit adds a 'flush' option to the createWriteStream()
family of functions.

Refs: #49886
PR-URL: #50093
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
This commit adds a 'flush' option to the createWriteStream()
family of functions.

Refs: nodejs#49886
PR-URL: nodejs#50093
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants