Node-style callbacks?

[pcothenet]

Hi team,

I've been looking at switching to the official client from this unoffical one.

The two main problems:

• MUST HAVE: The callback are not standard node-style callbacks (e.g. function(err, result)). One advantage of the unoffical intercom.io was to reject the promise (or return an err in the callback) when the response body would contain an error. In my opinion, this is one of the main advantages of using a node wrapper vs. the standard API.
• SHOULD HAVE: Support for promises (in addition to callbacks) would be really great (especially since this seems to be a feature of ES6)

Sticking to the other library in the meantime!

[bobjflong]

Hi @pcothenet

Thanks for getting in touch. We're still very much developing this lib, and we're hopeful we can get it to something everyone really loves using.

Re. your first point, would you mind checking out this diff? #18

[pcothenet]

That looks way better yes! (I don't think the doc is up-to-date though).

No worries, I'm actually super happy that you guys are building an official library. Happy to help!

[bobjflong]

@pcothenet docs updated with a note about calllbacks https://www.npmjs.com/package/intercom-client

[tejasmanohar]

👍 for what @pcothenet said. If you're going to expose callback-style functions, it should really follow node-style callbacks. The great part of this is users of the API can even integrate things like Bluebird's promisify() with their code to wrap your library if the convention is followed :D

[bobjflong]

@tejasmanohar when you say "node-style" are you referring to promises too? I may split out promises into another issue and close this to reduce confusion if so.

[tejasmanohar]

@bobjflong Nope, not at all. function(err, result) = node-style callbacks. My comment on Bluebird was related to Bluebird's promisify method which only works for node-style callbacks. Anyhow, node-style is the way to go due to convention :) - I have a few points of feedback on this repo and will send PRs soon.

[bobjflong]

@tejasmanohar but the callbacks currently support the form function(err, result) - see #18

[tejasmanohar]

Ah ok perfect, sorry, I was looking at the wrong thing. Thanks for clarifying, @bobjflong! I would recommend opening another issue for promises as well then as this is a bit misleading in title.

[bobjflong]

Done here: #45 I will likely add this today

[tejasmanohar]

Thanks, @bobjflong! Till then, users should be able to promisify this using a module like Bluebird or an independent one on NPM to get the same end result :)