search

alexwhitman / node-pushbullet-api

PushBullet API module for Node.js
165 stars 36 forks source link

Add promise support and tests #30

Closed grantholle closed 6 years ago

grantholle commented 6 years ago

There's support to use es6 native promises in lieu of callbacks. Callbacks are still supported, though. I also added tests that covers most of the features (although not all) using both callbacks and promises. I also discovered a few functions didn't work right (I think mostly the delete functions), and those are now working properly as well.

alexwhitman commented 6 years ago

Thanks for this. I haven't had time to look at it properly yet but I will get around to it soon.

grantholle commented 6 years ago

Have you had a chance to check it out yet?

grantholle commented 6 years ago

Made the requested changes, and also fixed #31 while I was in there which provides support for the body parameter on the link method.

alexwhitman commented 6 years ago

Sorry, something else I've just noticed. You've used ES6 default function parameters in a few places. These are only supported from 6.x. 4.x is still in LTS until April so I'd like to keep compatibility with that. The same goes for the use of const/let. Once 4.x is end of life the code can probably be cleaned up a bit to take advantage of new newer features.

I've just merged the use of eslint and Travis to catch these in future. This branch is will need rebasing as it now conflicts.

grantholle commented 6 years ago

So I was going to try and get everything compatible but...

Should es6: true be added to the eslint's env property? Embarrassingly I'm not too familiar with eslint 🙈

I'd hate to wait until April but I might. Thoughts?

dbuezas commented 6 years ago

I'd love if this could be published

alexwhitman commented 6 years ago

@grantholle I've had some time to look at this. It's close to being mergeable but when running the 'should send clipboard using a callback' test it throws an UnhandledPromiseRejectionWarning. We shouldn't be getting those when using the callback version.

grantholle commented 6 years ago

Ok, that should be working as expected now.

alexwhitman commented 6 years ago

Thanks @grantholle. I've squashed and merged this in https://github.com/alexwhitman/node-pushbullet-api/commit/504da45c26aad20e9f7bd9abb3ff4e3307ce329d

Version 2.1.0 will be released with this.