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

Remove rimraf dependency in favor of Node native rm #602

Merged
merged 8 commits into from
Nov 26, 2024
Merged

Conversation

niik
Copy link
Member

@niik niik commented Nov 20, 2024

#601 is open because of our use of rimraf which carries with it a ton of sub dependencies. I'm hoping to be able to get rid of it in favor of the recursive option for the native rm function from the fs module.

@niik niik enabled auto-merge November 20, 2024 14:01
@niik niik requested review from sergiou87 and tidy-dev November 20, 2024 14:01
@@ -1,43 +1,23 @@
import { spawn } from 'child_process'
import { glob } from 'glob'
Copy link
Member Author

Choose a reason for hiding this comment

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

So turns out I was depending on a sub-dependency of rimraf here so when I removed it glob disappeared as well but that lead to a nice cleanup in this script as well, utilizing the recursive option of readdir

Copy link
Member

Choose a reason for hiding this comment

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

Nice cleanup indeed 🥹

Copy link
Member

@sergiou87 sergiou87 left a comment

Choose a reason for hiding this comment

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

🔥

@niik niik merged commit 9979d25 into main Nov 26, 2024
6 checks passed
@niik niik deleted the rm-rf-rimraf branch November 26, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants