Skip to content

fs: correctly pass dirent to exclude withFileTypes - #53823

Closed
avivkeller wants to merge 3 commits into
nodejs:mainfrom
avivkeller:fix-dirent
Closed

fs: correctly pass dirent to exclude withFileTypes#53823
avivkeller wants to merge 3 commits into
nodejs:mainfrom
avivkeller:fix-dirent

Conversation

@avivkeller

@avivkelleravivkeller commented Jul 12, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Fixes#53821

Fixes an issue where some calls to #exclude did not pass the dirent when withFileTypes: true

CC @targos

@nodejs-github-botnodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Jul 12, 2024
@avivkelleravivkeller mentioned this pull request Jul 12, 2024
Closed
benjamingr
benjamingr reviewed Jul 12, 2024
View reviewed changes
Comment threadtest/parallel/test-fs-glob.mjs Outdated
@targos

targos commented Jul 12, 2024

Copy link
Copy Markdown
Member

direct -> dirent

@avivkelleravivkeller changed the title fs: correctly pass direct to exclude withFileTypesfs: correctly pass dirent to exclude withFileTypesJul 12, 2024
@avivkeller

avivkeller commented Jul 12, 2024

Copy link
Copy Markdown
MemberAuthor

@benjamingr Ive undone the common.mustCall, as the exclude function isn't called for every glob pattern that is being checked.

@avivkeller
avivkeller marked this pull request as draft July 12, 2024 21:36
@avivkeller

avivkeller commented Jul 12, 2024•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Removed the last common.mustCall, so this should be good, assuming all tests pass

@avivkeller
avivkeller marked this pull request as ready for review July 12, 2024 21:37
@avivkeller

avivkeller commented Jul 13, 2024

Copy link
Copy Markdown
MemberAuthor

All tests pass, can someone approve and run a Jenkins ci?

Thanks!

@MoLowMoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 14, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 14, 2024
@nodejs-github-bot

nodejs-github-bot commented Jul 14, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Jul 15, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Jul 17, 2024

Copy link
Copy Markdown
Collaborator

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

@avivkeller

avivkeller commented Jul 17, 2024

Copy link
Copy Markdown
MemberAuthor

All CIs passed :-). The two "failed" tests are just stopped linters.

@MoLowMoLow 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. labels Jul 17, 2024
@nodejs-github-botnodejs-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 Jul 17, 2024
@nodejs-github-bot

nodejs-github-bot commented Jul 17, 2024

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/53823
✔ Done loading data for nodejs/node/pull/53823
----------------------------------- PR info ------------------------------------
Title fs: correctly pass dirent to exclude `withFileTypes` (#53823)
⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch RedYetiDev:fix-dirent -> nodejs:main
Labels fs, needs-ci, commit-queue-squash
Commits 3
- fs: correctly pass dirent to exclude `withFileTypes`
- Update test-fs-glob.mjs
- Update test-fs-glob.mjs
Committers 2
- RedYetiDev <38299977+RedYetiDev@users.noreply.github.com>
- GitHub <noreply@github.com>
PR-URL: https://github.com/nodejs/node/pull/53823
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53823
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
--------------------------------------------------------------------------------
ℹ This PR was created on Fri, 12 Jul 2024 14:00:55 GMT
✔ Approvals: 3
✔ - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/53823#pullrequestreview-2175131131
✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/53823#pullrequestreview-2175259315
✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/53823#pullrequestreview-2176695952
✘ Last GitHub CI failed
ℹ Last Full PR CI on 2024-07-17T06:37:36Z: https://ci.nodejs.org/job/node-test-pull-request/60378/
- Querying data for job/node-test-pull-request/60378/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/9974836012

@MoLowMoLow removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jul 17, 2024
@avivkeller

avivkeller commented Jul 17, 2024•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

@MoLow can you rerun the stopped actions so the bot counts the GitHub CI as a pass? (lint-readme and lint-pr-url)

MoLow pushed a commit that referenced this pull request Jul 17, 2024
@avivkeller@MoLow
fs: correctly pass dirent to exclude withFileTypes
5090166
PR-URL: #53823
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@MoLow

MoLow commented Jul 17, 2024

Copy link
Copy Markdown
Member

Landed in 5090166

@MoLowMoLow closed this Jul 17, 2024
ehsankhfr pushed a commit to ehsankhfr/node that referenced this pull request Jul 18, 2024
@avivkeller@ehsankhfr
fs: correctly pass dirent to exclude withFileTypes
06e710f
PR-URL: nodejs#53823
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
targos pushed a commit that referenced this pull request Jul 28, 2024
@avivkeller@targos
fs: correctly pass dirent to exclude withFileTypes
b377b93
PR-URL: #53823
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@RafaelGSSRafaelGSS mentioned this pull request Jul 30, 2024
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benjamingrbenjamingrbenjamingr approved these changes
@targostargostargos approved these changes
@MoLowMoLowMoLow approved these changes

Assignees

No one assigned

Labels

commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.fsIssues and PRs related to the fs subsystem / file system.needs-ciPRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fsPromises.glob doesn't call exclude with dirent when called withFileTypes: true

5 participants

@avivkeller@targos@nodejs-github-bot@MoLow@benjamingr