Skip to content
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

write: (name: string) -> Promise<mixed> #645

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

krainboltgreene
Copy link

Promises are a great form of handling concurrency where a single value is passed around. Instead of doing a callback, now users can chain then's or catch's.

@ncuillery
Copy link

Hey I'd love to see Promise support for the write method.

I'm not a maintainer, but I review the code and I think the Promise support would better be optional:

  • If a callback is provided, the write method works just like before (no breaking change)
  • It no callback is provided, the writemethod returns a Promise

Lots of libraries works like that.

@krainboltgreene
Copy link
Author

The only failing bit is node 0.10, which frankly...it's way past lts.

@addisonElliott
Copy link

Came across this same problem and I agree with @ncuillery with the logic implementation. Quite a few libraries do handle it like that and it's intuitive to most users.

In the meantime until this PR is accepted, an applicable solution is to promisify the function and save it in gm, like so. Just add it to the top of your code before running any code using gm. Just use the writePromise function and it will return a promise instead.

gm.prototype.writePromise = util.promisify(gm.prototype.write);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants