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

Fix setOptions to complete the Future (previously did not call result.success) and remove side effects in example app #222

Merged
merged 8 commits into from Nov 22, 2021
Merged

Fix setOptions to complete the Future (previously did not call result.success) and remove side effects in example app #222

merged 8 commits into from Nov 22, 2021

Conversation

ben-xD
Copy link
Contributor

@ben-xD ben-xD commented Nov 12, 2021

A few small things:

  • I fix a bug in channelOptions: I now finish the platform method execution (complete the Future returned by setOptions)
  • I remove side effect of publishing messages when you launch the app. There are buttons for this functionality, and it confusing to see these messages get automatically sent when the app is launched.

@ben-xD ben-xD added bug Something isn't working. It's clear that this does need to be fixed. enhancement New feature or improved functionality. labels Nov 12, 2021
@ben-xD ben-xD self-assigned this Nov 12, 2021
@ben-xD ben-xD changed the title Example app: rest state was not being set. setOptions was not returning (because Java did not call result.success) and remove side effects in example app Nov 12, 2021
@ben-xD ben-xD changed the title setOptions was not returning (because Java did not call result.success) and remove side effects in example app Fix setOptions to complete the Future (previously did not call result.success) and remove side effects in example app Nov 12, 2021
@github-actions github-actions bot temporarily deployed to staging/pull/222/dartdoc November 12, 2021 16:51 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/222/dartdoc November 15, 2021 12:16 Inactive
@ben-xD ben-xD changed the base branch from main to enhancement/ios/notification-in-remote-message November 17, 2021 17:27
@github-actions github-actions bot temporarily deployed to staging/pull/222/dartdoc November 17, 2021 17:41 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/222/dartdoc November 18, 2021 09:57 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/222/dartdoc November 18, 2021 10:22 Inactive
ikbalkaya
ikbalkaya approved these changes Nov 18, 2021
View reviewed changes
Copy link
Contributor

@ikbalkaya ikbalkaya left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

Base automatically changed from enhancement/ios/notification-in-remote-message to main November 22, 2021 16:42
@QuintinWillison QuintinWillison merged commit 18a4369 into main Nov 22, 2021
@QuintinWillison QuintinWillison deleted the example-app-refactor branch November 22, 2021 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ikbalkaya ikbalkaya ikbalkaya approved these changes

@QuintinWillison QuintinWillison QuintinWillison approved these changes

@tiholic tiholic Awaiting requested review from tiholic

Assignees

@ben-xD ben-xD

Labels
bug Something isn't working. It's clear that this does need to be fixed. enhancement New feature or improved functionality.
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ben-xD @ikbalkaya @QuintinWillison