Skip to content

Replace mkdirp in tests with native JS - #743

Merged
thornjad merged 2 commits into
masterfrom
remove-mkdirp
Oct 11, 2021
Merged

thornjad merged 2 commits into
masterfrom
remove-mkdirp

Conversation

@thornjad

@thornjad thornjad commented Oct 6, 2021

Copy link
Copy Markdown
Member

mkdirp is one of those borderline useless packages. This PR replaces it with regular Node fs calls.

@thornjad thornjad added the patch version Small, non-breaking, bug fix or trivial change label Oct 6, 2021
Comment thread test/core-error.test.js
const baseDir = 'base';

mkdirp.sync(`${root}/emptyDir`);
require('fs').mkdirSync(`${root}/emptyDir`, {recursive: true});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For consistency you could have done something like

const { mkdirSync } = require('fs');
...
mkdirSync(`${root}/emptyDir`, {recursive: true});

Not a big deal, just a bit less noisy and the code would have looked similar to before.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I thought about it, but settled on just containing it to one line

@thornjad
thornjad merged commit 8647245 into master Oct 11, 2021
@thornjad
thornjad deleted the remove-mkdirp branch October 11, 2021 15:52
@thornjad thornjad added this to the v14.0 milestone Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement patch version Small, non-breaking, bug fix or trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants