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

ensure for link & symlink #165

Merged

Conversation

reggi
Copy link
Contributor

@reggi reggi commented Aug 6, 2015

No description provided.

@reggi
Copy link
Contributor Author

reggi commented Aug 6, 2015

@jprichardson

Added:

  • error messages use lstat and replace the string lstat with ensureSymlink and ensureLink respectivley
  • will not create recursive destination directories on error
  • branched out feature and dev so you can see diffs between pull-requests here

@reggi
Copy link
Contributor Author

reggi commented Aug 6, 2015

Should an error be thrown if the destination path exists?

@reggi reggi force-pushed the feature/ensurelink-ensuresymlink branch 2 times, most recently from dbd7911 to 3040283 Compare August 6, 2015 17:15
@reggi reggi force-pushed the feature/ensurelink-ensuresymlink branch from 3040283 to 66dc67f Compare August 6, 2015 17:16
jprichardson added a commit that referenced this pull request Aug 6, 2015
@jprichardson jprichardson merged commit 3ac543e into jprichardson:master Aug 6, 2015
@jprichardson
Copy link
Owner

Should an error be thrown if the destination path exists?

I don't think so.

Thanks for your hard work on this, I really appreciate it! Published 0.23.0.

@reggi
Copy link
Contributor Author

reggi commented Aug 6, 2015

The functions currently return an error for async and throw for sync if the destination path exists because that's the native functionality of fs.link and fs.symlink is exposed.

@jprichardson
Copy link
Owner

Yeah, I think that they should be normalized to not throw errors if the destination exists. I mean, the intent of the caller of these methods is to just make sure that they exist regardless of whether they existed before said method was called. Don't you think?

@reggi
Copy link
Contributor Author

reggi commented Aug 6, 2015

As expressed here I feel weird throwing the err away. My current usage for this function is as follows, please forgive the promises.

fse.ensureLinkAsync(pkg.main, paths.mainDst)
.then(debugThen('linking main %s -> %s', pkg.main, paths.mainDst)).catch(debugCatch),

So for control-flow it's useful to know if something has been created or not.

Thats why I expressed the need for ensureWarn functions.

I agree though with this:

the intent of the caller of these methods is to just make sure that they exist regardless

Crafting PR(s) now.

@reggi
Copy link
Contributor Author

reggi commented Aug 6, 2015

#169

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