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.

Sign up for GitHub

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

Jump to bottom

Checked for nullity of activation / deactivation results #303

Merged
merged 10 commits into from Jan 24, 2022
Merged

Checked for nullity of activation / deactivation results #303

merged 10 commits into from Jan 24, 2022

Conversation

ikbalkaya
Copy link
Contributor

@ikbalkaya ikbalkaya commented Jan 24, 2022 •
edited

This PR fixes #298

resultForActivate and resultForDeactivate will be checked before methods using them are invoked

@github-actions github-actions bot temporarily deployed to staging/pull/303/dartdoc January 24, 2022 11:27 Inactive
@ikbalkaya ikbalkaya linked an issue Jan 24, 2022 that may be closed by this pull request
Customer reporting issue with NPE on Android push notifications #298
Closed
KacperKluka
KacperKluka suggested changes Jan 24, 2022
View reviewed changes
Copy link

@KacperKluka KacperKluka left a comment

Choose a reason for hiding this comment

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

One change required and please reformat the code as some whitespace are missing 😉

android/src/main/java/io/ably/flutter/plugin/push/PushActivationEventHandlers.java Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging/pull/303/dartdoc January 24, 2022 11:47 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/303/dartdoc January 24, 2022 11:51 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/303/dartdoc January 24, 2022 11:56 Inactive
ikbalkaya and others added 2 commits January 24, 2022 11:56
@ikbalkaya @QuintinWillison
Update debug log for PUSH_DEACTIVATE_ACTION
61e868d 
Co-authored-by: Quintin Willison <q@qwuk.net>
@ikbalkaya @QuintinWillison
Update debug log for PUSH_ACTIVATE_ACTION
1b44309 
Co-authored-by: Quintin Willison <q@qwuk.net>
@github-actions github-actions bot temporarily deployed to staging/pull/303/dartdoc January 24, 2022 12:00 Inactive
KacperKluka
KacperKluka suggested changes Jan 24, 2022
View reviewed changes
Copy link

@KacperKluka KacperKluka left a comment

Choose a reason for hiding this comment

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

Just a small cosmetic change 😉

android/src/main/java/io/ably/flutter/plugin/push/PushActivationEventHandlers.java Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging/pull/303/dartdoc January 24, 2022 12:08 Inactive
@ikbalkaya
Added whitespaces for if/else blocks for cases PUSH_ACTIVATE_ACTION a…
1515418 
…nd PUSH_DEACTIVATE_ACTION
@ikbalkaya
Merge remote-tracking branch 'origin/bug/android_push_npe_channel' in…
93ecf46 
…to bug/android_push_npe_channel

# Conflicts:
#	android/src/main/java/io/ably/flutter/plugin/push/PushActivationEventHandlers.java
@ikbalkaya
Added whitespaces for if/else blocks for cases PUSH_ACTIVATE_ACTION a…
f9534e5 
…nd PUSH_DEACTIVATE_ACTION
KacperKluka
KacperKluka approved these changes Jan 24, 2022
View reviewed changes
Copy link

@KacperKluka KacperKluka left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@QuintinWillison
Copy link
Contributor

Looks like flakey tests explaining why Android 29 integration test run failed. We've got a customer waiting on this fix so I'm going to land this PR regardless so we can make a release.

@QuintinWillison QuintinWillison merged commit 0b0a8d8 into main Jan 24, 2022
@QuintinWillison QuintinWillison deleted the bug/android_push_npe_channel branch January 24, 2022 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuintinWillison QuintinWillison QuintinWillison approved these changes

@KacperKluka KacperKluka KacperKluka approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Customer reporting issue with NPE on Android push notifications
3 participants
@ikbalkaya @QuintinWillison @KacperKluka