-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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: fix bufferSize option for opendir recursive #55744
Conversation
89d3789
to
afc6294
Compare
afc6294
to
09a9ce9
Compare
09a9ce9
to
3d1821d
Compare
Could you make sure to document the issues with this new code (not sure if it's best put in comments or as a ticket)? I assume it's something along the lines of |
I'll add a comment and open a new issue 👍 |
The bufferSize option was not respected in recursive mode. This PR implements a naive solution to fix this issue until a better implementation can be designed. Fixes: nodejs#48820
3d1821d
to
75b300a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #55744 +/- ##
=======================================
Coverage 88.40% 88.40%
=======================================
Files 654 654
Lines 187747 187756 +9
Branches 36127 36128 +1
=======================================
+ Hits 165972 165985 +13
- Misses 15009 15010 +1
+ Partials 6766 6761 -5
|
CI is passing except for some flaky tests as far as I can tell. |
Commit Queue failed- Loading data for nodejs/node/pull/55744 ✔ Done loading data for nodejs/node/pull/55744 ----------------------------------- PR info ------------------------------------ Title fs: fix bufferSize option for opendir recursive (#55744) Author Ethan Arrowood <[email protected]> (@Ethan-Arrowood) Branch Ethan-Arrowood:fix/issue-48820 -> nodejs:main Labels fs, test, needs-ci Commits 1 - fs: fix bufferSize option for opendir recursive Committers 1 - Ethan Arrowood <[email protected]> PR-URL: https://github.com/nodejs/node/pull/55744 Fixes: https://github.com/nodejs/node/issues/48820 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/55744 Fixes: https://github.com/nodejs/node/issues/48820 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last approving review: ⚠ - fs: fix bufferSize option for opendir recursive ℹ This PR was created on Wed, 06 Nov 2024 12:12:12 GMT ✔ Approvals: 2 ✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/55744#pullrequestreview-2420753399 ✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/55744#pullrequestreview-2420811043 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-11-08T12:03:25Z: https://ci.nodejs.org/job/node-test-pull-request/63487/ - Querying data for job/node-test-pull-request/63487/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/11742390923 |
Landed in e0ef622 |
Should we consider backporting this since it is a bug fix affecting all current versions? |
The bufferSize option was not respected in recursive mode. This PR implements a naive solution to fix this issue until a better implementation can be designed. Fixes: #48820 PR-URL: #55744 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
The bufferSize option was not respected in recursive mode. This PR
implements a naive solution to fix this issue until a better
implementation can be designed.
Fixes: #48820