search

mailjet / mailjet-apiv3-nodejs

[API v3] Official Mailjet API v3 NodeJS wrapper
https://dev.mailjet.com
MIT License
236 stars 69 forks source link

EventEmitter ≠ Promise #7

Closed p-j closed 8 years ago

p-j commented 8 years ago

Hi there,

I'm a bit confused with the documentation/readme :

Why would you say Make the same request with a promise or call it a promise when it's clearly not a promise ?

I would suggest to either go with promise and use something like petkaantonov/bluebird to return a thenable or stay with the EventEmitter pattern and stop advertising it as something it isn't.

I've made the required change for a proper Promise implementation (both code & test) along with consistency fix (code style & line endings). I've used standardjs as code style because it is what I use, but if you'd rather keep semicolons you can checkout semistandard and LF line endings instead of mixed CR & LF.

If you like it I can make a pull request, I've already bumped the major version in the package.json as moving away from EventEmitter is a breaking change.

Either way, thanks for putting together this package as a starting point for using Mailjet API :-)

WeshGuillaume commented 8 years ago

Hey !

Yes right, At the time, there were no stable implementation of the Promise API, but yes you are right, I think you can open a pull request with a major version update in the package.json, it will be merged!

Thanks, Guillaume

p-j commented 8 years ago

Just did :-) Thanks for the quick reply !