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

Add more log for push #249

Merged
merged 1 commit into from
Dec 2, 2015

Conversation

wangmengyan95
Copy link
Contributor

  1. Add log when we recive deviceToken from GCM.
  2. Add log when developers try to use PPNS but miss something.
  3. Add log to recommend developers to use GCM

@wangmengyan95
Copy link
Contributor Author

@hallucinogen it would be great if you can also tay a look.

@facebook-github-bot
Copy link

By analyzing the blame information on this pull request, we identified @grantland and @wangmengyan95 to be potential reviewers.

@@ -260,6 +261,40 @@ public static PushType getPushType() {
PLog.e(TAG, "Cannot use GCM for push because the app manifest is missing some " +
"required declarations. Please " + getGcmManifestMessage());
}
} else if (hasAnyPPNSSpecificDeclaration) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we clean this up on purpose. We used to have this flag. @grantland

@wangmengyan95 wangmengyan95 force-pushed the wangmengyan.update_detail_log_about_push branch from 5feac86 to bfd9651 Compare November 20, 2015 22:23
@facebook-github-bot
Copy link

@wangmengyan95 updated the pull request.

@@ -138,7 +138,9 @@ public void onReceive(Context context, Intent intent) {
protected void onPushReceive(Context context, Intent intent) {
JSONObject pushData = null;
try {
pushData = new JSONObject(intent.getStringExtra(KEY_PUSH_DATA));
String pushDataStr = intent.getStringExtra(KEY_PUSH_DATA);
PLog.v(TAG, pushDataStr);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure whether this introduces too much overhead for production, but for debugging purpose, I think this is necessary since there is no reliable way for us to tell whether a device has received a push notification or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be fine. We also did something similar on .NET.

Copy link
Contributor

Choose a reason for hiding this comment

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

"Received push data: " + pushDataStr

@hallucinogen
Copy link
Contributor

yay!

@wangmengyan95 wangmengyan95 force-pushed the wangmengyan.update_detail_log_about_push branch from bfd9651 to e8fc559 Compare December 2, 2015 19:59
@facebook-github-bot
Copy link

@wangmengyan95 updated the pull request.

@grantland
Copy link
Contributor

LGTM

wangmengyan95 added a commit that referenced this pull request Dec 2, 2015
@wangmengyan95 wangmengyan95 merged commit dfd52ab into master Dec 2, 2015
@wangmengyan95 wangmengyan95 deleted the wangmengyan.update_detail_log_about_push branch December 2, 2015 21:21
@grantland grantland modified the milestone: 1.11.1 Dec 17, 2015
@facebook-github-bot
Copy link

@wangmengyan95 updated the pull request.

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.

None yet

5 participants