-
Notifications
You must be signed in to change notification settings - Fork 776
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
[BREAKING] moveSync: refactor to use renameSync #609
Conversation
I changed the base to |
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.
Could not think of any addition. Also nice cleanups in the tests 👏
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.
Code LGTM
This will need rebased to fix merge conflicts. @manidlou you've been very helpful here at fs-extra, and too often, I feel like I'm the bottleneck with the release process. As a result, I've added you to fs-extra on npm (i.e. you now have |
@RyanZim thanks a lot for giving me publish rights. I am truly honored to be part of You are right about the conflict. I'll rebase this. |
3d9e8ac
to
95a9c72
Compare
Rebased! |
@manidlou Sorry, seems npm was a little buggy and didn't give you access when I requested it. Now fixed; you should have access. |
Thanks @RyanZim I appreciate it! |
@jprichardson are you ok with this?! So that I can merge this since |
@manidlou yep, I'm okay with it |
fix #608.
Refactored
moveSync
to be consistent withmove
. Also removed unnecessaryrimraf
package from dev-deps.