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

Adding initial promise end point #16

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

Conversation

jung-kim
Copy link

@jung-kim jung-kim commented Apr 10, 2018

fix: #15

WIP

  • Adding in package-lock.json per article at here.
  • Adding in Semaphore.waitAsync() that returns a promise
  • Adding in test. (Added random ness to the next tick to introduce some randomness in call order)

(will add implementations and tests to others when current impl is deemed acceptable)

@ronkorving
Copy link
Collaborator

ronkorving commented Apr 11, 2018

I'm not sure if I'm a total fan of postfixing all API with Async. In #15 I asked and was hoping you would have a few suggestions regarding what we could do here. I guess this PR is one suggestion :) Did you have any other ideas? I have some, but don't mind hearing your thoughts first.

@jung-kim
Copy link
Author

Yeah, sorry I'm bit shit at describing my thoughts and figured code is the best way. I don't mind evolving or scrapping this PR.

As fars as Async postfix, yes I do find it very strange and I'm not a fan of it myself. However, I'm not aware of any standards around this but Bluebird's promisify all function follows this convention and it is the only one that I'm aware that does this.

I personally would have preferred to called it ${function_name}Prom() format

@ronkorving
Copy link
Collaborator

Myself, I was thinking it may be nice to be able to get a promisified version of locks by calling:

const locks = require('locks/promise');

Or something like that. That's just one alternative, there may be more :)

@jung-kim
Copy link
Author

Been quite busy meself, and it is now fixed.

And Yeah, I would agree that doing such sub directory is a better idea. I've tried to reutilize existing logic so plea feel free to let me know.

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.

Native support for promises?
2 participants