cojohn / node-urban-airship

Simple wrapper for the Urban Airship API in Node.js.
MIT License
36 stars 15 forks source link

add support for registering and unregistering android devices #6

Closed fabiocionini closed 11 years ago

fabiocionini commented 11 years ago
cojohn commented 11 years ago

Could you modify your changes so that they are backwards compatible with previous versions of the API? For example, with unregisterDevice():

unregisterDevice(<token>, <device_type>, <callback>);

And check if device_type is a function with something like:

var callback = device_type instanceof Function ? device_type : callback;
var device_type = device_type instanceof Function ? "ios" : device_type;

As it stands this pull request would break implementations in my own work and likely others. Closing for now, but happy to accept a pull request that properly accounts for prior versions of the API.

fabiocionini commented 11 years ago

yep, sorry for that. I had to modify it for a project and pushed my changes without thinking about this :) I'll send you a pull request with backward-compatible changes.

Fabio Cionini TODO interaction design www.todo.to.it www.betternouveau.com @todotoit @fabiocionini

On 14/ago/2013, at 15:57, Christopher John notifications@github.com wrote:

Could you modify your changes so that they are backwards compatible with previous versions of the API? For example, with unregisterDevice():

unregisterDevice(, , ); ``

And check if device_type is a function with something like:

var callback = device_type instanceof Function ? device_type : callback; var device_type = device_type instanceof Function ? "ios" : "android";

As it stands this pull request would break implementations in my own work and likely others. Closing for now, but happy to accept a pull request that properly accounts for prior versions of the API. — Reply to this email directly or view it on GitHub.

cojohn commented 11 years ago

FTR, it doesn't have to be the solution I proposed, just something to allow for backwards compatibility. Changing device_id to an object that identifies the device:

{"id": "blah", "type":"ios"|"android"|"blackberry"}

might be a better long term solution, as you point out iOS and Android won't be the only devices people want to support.