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.

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: remove recursive option from rmdir #57784

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

LiviaMedeiros
Copy link
Member

This moves DEP0147: fs.rmdir(path, { recursive: true }) to EOL.

The recursive option was runtime deprecated since v16.0.0 in favor of rm() function. Maybe it is time to remove it.

cc @nodejs/fs because fs Issues and PRs related to the fs subsystem / file system.
cc @nodejs/tsc because semver-major PRs that contain breaking changes and should be released in the next major version.
cc @addaleax because of concerns in the runtime deprecation PR #37302 (comment)

@LiviaMedeiros LiviaMedeiros added fs Issues and PRs related to the fs subsystem / file system. semver-major PRs that contain breaking changes and should be released in the next major version. deprecations Issues and PRs related to deprecations. labels Apr 7, 2025
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 7, 2025
@LiviaMedeiros LiviaMedeiros added the needs-citgm PRs that need a CITGM CI run. label Apr 7, 2025
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link

codecov bot commented Apr 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.25%. Comparing base (33f6e1e) to head (f9034bc).
Report is 33 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #57784    +/-   ##
========================================
  Coverage   90.24%   90.25%            
========================================
  Files         630      630            
  Lines      185245   185136   -109     
  Branches    36299    36286    -13     
========================================
- Hits       167173   167090    -83     
+ Misses      11018    10988    -30     
- Partials     7054     7058     +4     
Files with missing lines Coverage Δ
lib/fs.js 98.25% <100.00%> (-0.02%) ⬇️
lib/internal/fs/promises.js 98.22% <ø> (-0.02%) ⬇️
lib/internal/fs/utils.js 99.68% <100.00%> (-0.01%) ⬇️

... and 39 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nodejs-github-bot
Copy link
Collaborator

@mcollina
Copy link
Member

There seem to be a lot of CITGM failures compared to main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecations Issues and PRs related to deprecations. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. needs-citgm PRs that need a CITGM CI run. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants