-
Notifications
You must be signed in to change notification settings - Fork 341
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 promise support using universalify #109
Conversation
Please remove the |
Done 😄 |
Hey i also added documentation in read-me. Could you please check that out too? |
Also should add tests for promise usage. |
@RyanZim sure i will add tests. I had one doubt though. |
I have added tests for promises, also have refactored existing tests so that common code can be shared among callback and promise tests (using beforeEach) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Released in 5.0.0 🎉 |
yaay 😄 🎉 |
I saw your comment on this pull request, #103.
I used universalify to add promisesupport. Must admit the solution looks very clean this way.
Could you please review this?
Things changed:
Could you tell me if i need to change something or if the changes look good to you.