Conversation
Signed-off-by: Charlike Mike Reagent <olsten.larck@gmail.com>
Signed-off-by: Charlike Mike Reagent <olsten.larck@gmail.com>
| if (checkTypes(task, ['sh', 'bash'])) { | ||
| return runCLICommand({ task, resolve, reject }) | ||
| runCLICommand({ task, resolve, reject }) | ||
| return |
There was a problem hiding this comment.
because it's more meaningful and it may be a bit confusing. we dont return promise from there anyway. and we dont need to return from there either because we are passing resolve and reject to the runCLICommand.
There was a problem hiding this comment.
also, return frome promise constructor make no sense, you are forced to use resolve and reject anyway. only if something fail in that constructor only then it goes to the catch handler, otherwise you are forced to use resolve and reject
in this case it will just throw , because we directly await the promise
|
I always thought that it's always a good practice to separate the return inside callbacks and always did that in the old days of callbacks. It just not make any sense. The promise constructor is a callback. |
fixes #6 -
lib/runJavaScript.jsis pretty cool, we should publish it to npm ;d I didn't found such thing, but i'm 99% that i seen such modules in the past year.And actually, i don't see how we can implement #11, even if we not merge this PR.