-
Notifications
You must be signed in to change notification settings - Fork 25
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
Node 10 requires a callback to rmdir #69
base: master
Are you sure you want to change the base?
Conversation
lib/phonegap-build/create/zip.js
Outdated
@@ -116,7 +116,7 @@ module.exports = { | |||
// remove zip directory if empty | |||
exists = fs.existsSync(basepath); | |||
if (exists) { | |||
fs.rmdir(basepath); | |||
fs.rmdir(basepath, function () {}); |
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.
Since the sync function is used above in fs.existsSync
, you might want to use fs.rmdirSync
instead.
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.
Yeah good point!
It would be nice if this could be merged so Node 10 is supported. |
+1 |
+1 please merge! |
Not ideal, but I solved this by just updating the source (as in PR) locally. https://github.com/phonegap/node-phonegap-build/issues/68 |
Wow still not merged? I've got the same problem today and this fixed it. |
I'm not so awesome at all this node/phonegap config stuff, and my remote build is failing as well. Can someone explain for us dummies how we'd go about using this patched gust42 version of phonegap build? |
you can find the file locally on your computer and make the changes, lib/phonegap-build/create/zip.js |
Need this also. Is the project dead/inactive? |
please merge this! this issue has existed for more than a year now! |
Makes it work with Node 10