feat: promisify the "run" command #269
Conversation
Almenon
left a comment
There was a problem hiding this comment.
Thanks for submitting the PR! It's a good start. If you want to I can make the below changes, or you can take a shot at them:
- Check your returns, some of them aren't needed.
- Having a callback in addition to a promise might be confusing to the person using the API. I'm assuming you left it in as to not cause breaking changes, which is good practice, but the return value is already changed, which means a major semver update. As long as we're making changes lets get rid of the callback param.
- for consistency it would be good to promisify the runstring method as well.
|
I believe I have found a way to support both the legacy callback-based API and the newer promise-based one.
|
|
Sorry for the delay, I do intend on getting to this eventually. |
|
The problem with trying to support both approaches is that the typing doesn't work out. If you use .then typescript complains because the return value might not be a promise. There might be some fancy typescript syntax for solving this, but I'm just going to simplify and remove the callback option for the next major release. I'm presuming that's fine? |
|
I'm going to merge this in, publish as v4 so you have your changes, and then make some changes I want to make w/ a v5 release. |
that's my humble attempt to solve issue #77
thanks for making and maintaining this repo