-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add APNS client #235
Add APNS client #235
Conversation
9ebce6c
to
81a5a8f
Compare
cc @fantasist for review |
* @param {Array} deviceTokens A array of device tokens | ||
* @returns {Object} A promise which is resolved immediately | ||
*/ | ||
APNS.prototype.send = function(data, deviceTokens) { |
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.
no max for deviceTokens
?
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.
No, internally the library just sends the notifications one by one, so we do not need to have size limitation.
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. One issue is how to deal with dev vs. prod device tokens. You'll probably need a higher layer that selects the appropriate APNs connection to use (dev or prod) based on the bundle id of the app (available in the "appIdentifier" field of the installation). |
Okay, seems fine to me. We can add the connection-selecting behavior in some higher-level module. |
No description provided.