Skip to content
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

Support send push notification by topic #74

Merged
merged 4 commits into from
Jan 6, 2017

Conversation

ben181231
Copy link
Contributor

@ben181231 ben181231 commented Jan 5, 2017

@oursky-travis
Copy link

@ben181231, thanks for your PR! By analyzing the history of the files in this pull request, we identified @cheungpat and @rickmak to be potential reviewers.

// Called when the application is about to terminate. Save data if appropriate. See also applicationDidEnterBackground:.
func application(application: UIApplication, didFailToRegisterForRemoteNotificationsWithError error: NSError) {
print("Failed to register remote notification: \(error.localizedDescription)")
self.registerDevice()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t understand when and why the user should call the registerDevice functions. Need much more doc in this file so that the developer knows what to do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to inline the registerDevice functions in the app delegate functions. That way the developer knows what to do in app delegate without referencing other functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay.

registerDevice() is registering a device to skygear with null device token. The device ID will be reused once the SDK can get the device token.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not sure what you mean, waiting for your update to this PR

@@ -22,7 +22,9 @@

@interface SKYRegisterDeviceOperation : SKYOperation

- (instancetype)initWithDeviceToken:(NSData *)deviceToken;
- (instancetype)initWithDeviceToken:(NSData *)deviceToken
__attribute__((deprecated("Use -(void)initWithDeviceToken:topic: instead")));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, i didn’t know about this
no need for the return type of the method here, and btw the return type of the method is incorrect.

Ditto for below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... I will remove the return type of the method here.

This is trying to mark this method to as deprecated.

@ben181231
Copy link
Contributor Author

Updated.

@cheungpat cheungpat merged commit 22698d1 into SkygearIO:master Jan 6, 2017
@ben181231 ben181231 deleted the provide-topic-name branch January 31, 2018 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants