-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
add concat-stream to devDependencies #353
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Can we replace rimraf
with fs.rmdirSync
?
@climba03003 I doubt it, if they are transitive dependencies, unless we send PRs upstream. |
I means it is added in It only used in one place inside the codebase. fastify-multipart/test/multipart-disk.test.js Line 248 in c04dcb7
|
ah, you are right. |
@kibertoad |
We could drop rimraf as devDependency by doing if (fs.promises.rm) {
await fs.promises.rm(tmpdir, { recursive: true, force: true })
} else {
await fs.promises.rmdir(tmpdir, { recursive: true })
} or await fs.promises.rm(tmpdir, { recursive: true, force: true }) fs.promises.rm is available since 14.14.0 What do you prefer? |
Since we test against the latest version of node (inside that major). |
@climba03003 done ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
Currently, rimraf and concat-stream are imported because they are dependencies of other packages. This PR adds them explicitly.