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.

Sign up for GitHub

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

Jump to bottom

fs: remove unnecessary option argument validation #53958

Merged
merged 3 commits into from
Aug 6, 2024
Merged

fs: remove unnecessary option argument validation #53958

merged 3 commits into from
Aug 6, 2024

Conversation

JonasBa
Copy link
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
lib/fs.js Outdated Show resolved Hide resolved
@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
Collaborator

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
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
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
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
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
lib/fs.js Outdated Show resolved Hide resolved
@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
Collaborator

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

@JonasBa
Copy link
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
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
Collaborator

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
52 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 3ed9f98

@JonasBa
Copy link
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 <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Aug 19, 2024
Merged
@targos targos added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Sep 21, 2024
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. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. 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