Promisify fs methods
#62
Conversation
|
Bump @sindresorhus ? |
index.js
Outdated
| }); | ||
| }) | ||
| .catch(err => fsOperationFailed(stream, sourceFile, err)) | ||
| .then(() => cb()); |
There was a problem hiding this comment.
The point of promisifying is so that we can drop the cb() here and in the function signature and just return a promise from the function.
|
Thanks for working on this :) |
|
No problem, thanks for making it in the first place :) |
index.js
Outdated
| .then(targetData => { | ||
| if (sourceFile.isNull()) { | ||
| cb(null, sourceFile); |
There was a problem hiding this comment.
Not sure if this is okay, but after Promise we run cb() either way?
There was a problem hiding this comment.
No. You still need to pass the file down the stream even though its contents is null.
There was a problem hiding this comment.
Right, thanks! Fixed it now.
index.js
Outdated
| @@ -86,7 +82,7 @@ module.exports = (dest, opts) => { | |||
| } | |||
| } | |||
|
|
|||
| opts.hasChanged(this, cb, file, newPath); | |||
| opts.hasChanged(this, file, newPath).then(() => cb()); | |||
There was a problem hiding this comment.
You can now move the fsOperationFailed error handling here instead of catching it in both compareLastModifiedTime and compareSha1Digest.
There was a problem hiding this comment.
Right! Thank you, and sorry for doing this line by line!
Will do in 20
|
Last thing that I forgot earlier. You need to update the |
|
@sindresorhus Yeah, I wasn't sure what's the accepted/usual open source way of doing changes in docs, and other non-code things. Should I go ahead and change the docs, and you'll do the rest afterwards? edited to sound less like I just woke up |
Only the docs. I'll do the up version when publishing a new version. |
|
@sindresorhus sorry, had a busy few days. Updated the readme. |
|
Thank you @adamkiss. I really appreciate this contribution :) |
I want to reuse this plugin elsewhere, so in the spirit of Open Source I took a crack at one of the 'help wanted' issues to familiarize myself with the codebase.
Please let me know your thoughts. Should close #59.