reject promise using Error and include error message #80
Conversation
develar
changed the title
|
Ping :) |
|
PR rebased. |
| debug(`Executing ${file} ${args == null ? '' : removePassword(args.join(' '))}`) | ||
| } | ||
|
|
||
| return new Promise((resolve, reject) => { |
There was a problem hiding this comment.
I would suggest using the old standard thus having better backward compatibility on node?
There was a problem hiding this comment.
Both electron-packager and electron-builder require nodejs 4+. So, I think, it is ok to use template strings.
There was a problem hiding this comment.
@develar would you help remove the Node support in in .travis.yml as well if we're dropping 0.12 and some others? Travis didn't pick up the error because it doesn't run through the actual code but a quick syntax/consistency check.
There was a problem hiding this comment.
Old nodejs version removed, node 7 added.
| module.exports.execFileAsync = Promise.promisify(child.execFile) | ||
| module.exports.execFileAsync = function (file, args, options) { | ||
| if (debug.enabled) { | ||
| debug(`Executing ${file} ${args == null ? '' : removePassword(args.join(' '))}`) |
There was a problem hiding this comment.
I'm not sure if we should call debuglog here as debug creates a namespace for logging.
There was a problem hiding this comment.
Yes, debuglog must be used here, I will fix it.
| @@ -1,2 +1,4 @@ | |||
| node_modules | |||
| test/work | |||
| .idea/ | |||
There was a problem hiding this comment.
@develar is this necessary in .gitignore? I'm not sure where it is applied.
There was a problem hiding this comment.
If you don't mind, I want to add it — it is IDEA/WebStorm project files directory. In case of this project I don't want to add my IDEA project to VCS but want to ignore it.
There was a problem hiding this comment.
Oh sure 👍 I don't mind having it just in case.
|
@develar merged. I'll update a few bits of the code regarding this change on |
plist updated to 2.0 — it is safe to do major upgrade