-
Notifications
You must be signed in to change notification settings - Fork 772
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
move: do not create parent directory if it is root #897
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, but one flaw with the cleanup code in the tests; they will not run properly if the tests fail.
// this test is a bit special since | ||
// we're moving to root, so we need to | ||
// manually remove dest after we're done. | ||
fse.removeSync(dest) |
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.
Should be in an afterEach
in this describeIfWindows
block.
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.
Makes sense! I will update it then.
lib/move/__tests__/move.test.js
Outdated
// this test is a bit special since | ||
// we're moving to root, so we need to | ||
// manually remove dest after we're done. | ||
fse.remove(dest, 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.
Same here.
Fixes #819.
Do not create parent directory if it is root. We run tests for this only on windows, moving to root on unix-like systems requires sudo privileges.