Skip to content

fs: remove unnecessary option argument validation#53958

Merged
nodejs-github-bot merged 3 commits intonodejs:mainfrom
JonasBa:jb/fs-mkdir-options
Aug 6, 2024
Merged

fs: remove unnecessary option argument validation#53958
nodejs-github-bot merged 3 commits intonodejs:mainfrom
JonasBa:jb/fs-mkdir-options

Conversation

@JonasBa
Copy link
Copy Markdown
Contributor

@JonasBa JonasBa commented Jul 19, 2024

Remove unnecessary option parsing in mkdir

@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 Jul 19, 2024
@JonasBa JonasBa force-pushed the jb/fs-mkdir-options branch from 6327d7b to f4e9f08 Compare July 19, 2024 21:24
anonrig
anonrig reviewed Jul 19, 2024
View reviewed changes
Comment thread lib/fs.js Outdated
@anonrig anonrig added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Jul 19, 2024
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 19, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 19, 2024
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Jul 19, 2024

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

@anonrig anonrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 19, 2024
@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented Jul 21, 2024

CI is failing

@aduh95 aduh95 removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 21, 2024
@anonrig
Copy link
Copy Markdown
Member

anonrig commented Jul 21, 2024

CI is failing

I guess we are also testing the execution order here. We should update the tests @JonasBa

@JonasBa
Copy link
Copy Markdown
Contributor Author

JonasBa commented Jul 26, 2024

Yeah, sorry about that. My device is managed and I cant change firewall rules to run the tests without getting a million connection popups

@anonrig
Copy link
Copy Markdown
Member

anonrig commented Jul 26, 2024

Yeah, sorry about that. My device is managed and I cant change firewall rules to run the tests without getting a million connection popups

You can run "tools/test.py path/to/test/file" for specific tests

anonrig
anonrig reviewed Aug 5, 2024
View reviewed changes
Comment thread lib/fs.js
@JonasBa JonasBa force-pushed the jb/fs-mkdir-options branch from 20a13bf to 89a14bf Compare August 5, 2024 20:30
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Aug 6, 2024

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

@JonasBa
Copy link
Copy Markdown
Contributor Author

JonasBa commented Aug 6, 2024

@anonrig is ti common for the build to run so long or did this timeout and needs a rerun? I dont see any failures, but I lack context on being able to tell how long this should take :)

@anonrig
Copy link
Copy Markdown
Member

anonrig commented Aug 6, 2024

@anonrig is ti common for the build to run so long or did this timeout and needs a rerun? I dont see any failures, but I lack context on being able to tell how long this should take :)

This is a timeout probably

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Aug 6, 2024

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

@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 6, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 6, 2024
@nodejs-github-bot nodejs-github-bot merged commit 3ed9f98 into nodejs:main Aug 6, 2024
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Aug 6, 2024

Landed in 3ed9f98

@JonasBa
Copy link
Copy Markdown
Contributor Author

JonasBa commented Aug 7, 2024

Thank you @anonrig 🙏🏼

targos pushed a commit that referenced this pull request Aug 14, 2024
@JonasBa @targos
fs: remove unnecessary option argument validation
112228c 
PR-URL: #53958
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@RafaelGSS RafaelGSS mentioned this pull request Aug 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anonrig anonrig anonrig approved these changes

Assignees

No one assigned

Labels

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. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@JonasBa @nodejs-github-bot @aduh95 @anonrig @targos
⬇