node icon indicating copy to clipboard operation
node copied to clipboard

fs: acknowledge `signal` option in `filehandle.createReadStream()`

Open LiviaMedeiros opened this issue 1 year ago • 5 comments

filehandle.createReadStream({ signal }) is supported but it was not documented nor tested.

LiviaMedeiros avatar Sep 28 '24 05:09 LiviaMedeiros

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.24%. Comparing base (b5fb2ff) to head (39bcf3c). Report is 54 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #55148      +/-   ##
==========================================
- Coverage   88.25%   88.24%   -0.01%     
==========================================
  Files         651      651              
  Lines      183898   183937      +39     
  Branches    35860    35860              
==========================================
+ Hits       162298   162316      +18     
- Misses      14893    14908      +15     
- Partials     6707     6713       +6     

see 37 files with indirect coverage changes

codecov[bot] avatar Sep 28 '24 06:09 codecov[bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62822/

nodejs-github-bot avatar Sep 28 '24 12:09 nodejs-github-bot

Do we know when was this added? If so, could we add an entry in the YAML list?

aduh95 avatar Sep 28 '24 23:09 aduh95

AFAICT it supported signal since beginning when this function was added, so there's no need in separate entry. Unless we want something like "officially supports since REPLACEME"

LiviaMedeiros avatar Sep 29 '24 16:09 LiviaMedeiros

CI: https://ci.nodejs.org/job/node-test-pull-request/62848/

nodejs-github-bot avatar Sep 29 '24 18:09 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/62851/

nodejs-github-bot avatar Sep 30 '24 08:09 nodejs-github-bot

Commit Queue failed
- Loading data for nodejs/node/pull/55148
✔  Done loading data for nodejs/node/pull/55148
----------------------------------- PR info ------------------------------------
Title      fs: acknowledge `signal` option in `filehandle.createReadStream()` (#55148)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     LiviaMedeiros:fs-acknowledge-createReadStream-signal -> nodejs:main
Labels     doc, fs, test, author ready, needs-ci
Commits    2
 - fs: acknowledge `signal` option in `filehandle.createReadStream()`
 - squash: add more tests
Committers 1
 - LiviaMedeiros <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/55148
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55148
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 28 Sep 2024 05:27:39 GMT
   ✔  Approvals: 2
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/55148#pullrequestreview-2335388551
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/55148#pullrequestreview-2335904986
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-09-30T08:51:49Z: https://ci.nodejs.org/job/node-test-pull-request/62851/
- Querying data for job/node-test-pull-request/62851/
   ✔  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
✔  origin/main is now up-to-date
- Downloading patch for 55148
From https://github.com/nodejs/node
 * branch                  refs/pull/55148/merge -> FETCH_HEAD
✔  Fetched commits as 6b9413e41add..39bcf3c8fdaf
--------------------------------------------------------------------------------
Auto-merging doc/api/fs.md
[main fc83f845e1] fs: acknowledge `signal` option in `filehandle.createReadStream()`
 Author: LiviaMedeiros <[email protected]>
 Date: Sat Sep 28 13:20:24 2024 +0800
 2 files changed, 20 insertions(+)
[main e8f99e9707] squash: add more tests
 Author: LiviaMedeiros <[email protected]>
 Date: Sun Sep 29 01:31:25 2024 +0800
 1 file changed, 53 insertions(+)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)

Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- fs: acknowledge signal option in filehandle.createReadStream()

PR-URL: https://github.com/nodejs/node/pull/55148 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

[detached HEAD 189628f3c8] fs: acknowledge signal option in filehandle.createReadStream() Author: LiviaMedeiros <[email protected]> Date: Sat Sep 28 13:20:24 2024 +0800 2 files changed, 20 insertions(+) Rebasing (3/4) Rebasing (4/4)

Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- squash: add more tests

PR-URL: https://github.com/nodejs/node/pull/55148 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

[detached HEAD c06edf1705] squash: add more tests Author: LiviaMedeiros <[email protected]> Date: Sun Sep 29 01:31:25 2024 +0800 1 file changed, 53 insertions(+)

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/11156869043

nodejs-github-bot avatar Oct 03 '24 06:10 nodejs-github-bot

Landed in 36ca010bef4f869d7a6aa2d496aa92cd7c4f0e7d

nodejs-github-bot avatar Oct 03 '24 06:10 nodejs-github-bot