Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[connectivity] Migrate more logic to method channel platform#2526

Merged
ditman merged 11 commits intoflutter:masterfrom
franciscojma86:d-conn
Feb 14, 2020
Merged

[connectivity] Migrate more logic to method channel platform#2526
ditman merged 11 commits intoflutter:masterfrom
franciscojma86:d-conn

Conversation

@franciscojma86
Copy link
Contributor

@franciscojma86 franciscojma86 commented Feb 13, 2020

Description

An improvement over the current platform_interface implementation, migrating more logic into the method channel platform.

Related Issues

flutter/flutter#46307

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See [Contributor Guide]).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the [Flutter Style Guide].
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the [CLA].
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

franciscojma86 and others added 9 commits February 7, 2020 14:51
…plementation.

connectivity_platform_interface:

* Bring ConnectivityResult and LocationAuthorizationStatus enums from
core package.
* Use the above Enums as return values for ConnectivityPlatformInterface
methods.
* Modify the MethodChannel implementation so it returns the right types.
* Bring utility methods, asserts and other logic that is only needed on
the MethodChannel implementation from the core package, so it's simpler.
* Bring MethodChannel unit tests from core package.

connectivity (core pkg):

* Reexport ConnectivityResult and LocationAuthorizationStatus enums so
plugin users can see them.
* Remove MethodChannel unit tests (moved to platform_interface).
@franciscojma86
Copy link
Contributor Author

Mostly @ditman work, but happy to take the credit :)

@ditman
Copy link
Member

ditman commented Feb 13, 2020

@franciscojma86 you can't push changes to two packages at the same time, because our testing won't know what you do.

You first need to PR and release the changes to the platform_interface, and then the changes to the core plugin to use the new platform interface. That way you can use the correct version in the dependency of your core plugin.

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

Please, remove changes from the core connectivity package, so this PR just changes the connectivity_platform_interface package.

}) {
// `assert(Platform.isIOS)` will prevent us from doing dart side unit testing.
// TODO: These should noop for non-Android, instead of throwing, so people don't need to rely on dart:io for this.
assert(!Platform.isAndroid);
Copy link
Member

Choose a reason for hiding this comment

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

I thought about this a little bit, and we need to remove these Platform asserts, they don't add anything to this plugin (other than a dependency in dart:io).

We can do this in other PR, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I thought so too, but I think the first changes I'd like to keep the API as similar as possible to the current behavior.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, sounds good to me!

@ditman ditman merged commit ff7838d into flutter:master Feb 14, 2020
@franciscojma86 franciscojma86 changed the title [connectivity] Use platform_interface in the core plugin [connectivity] Migrate more logic to method channel platform Feb 14, 2020
sanekyy pushed a commit to sanekyy/plugins that referenced this pull request Feb 18, 2020
…from the core package (flutter#2526)

* Bring ConnectivityResult and LocationAuthorizationStatus enums from
core package.
* Use the above Enums as return values for ConnectivityPlatformInterface
methods.
* Modify the MethodChannel implementation so it returns the right types.
* Bring utility methods, asserts and other logic that is only needed on
the MethodChannel implementation from the core package, so it's simpler.
* Bring MethodChannel unit tests from core package.

Co-authored-by: David Iglesias <ditman@gmail.com>
EdwinRomelta pushed a commit to EdwinRomelta/plugins that referenced this pull request Jun 11, 2020
…from the core package (flutter#2526)

* Bring ConnectivityResult and LocationAuthorizationStatus enums from
core package.
* Use the above Enums as return values for ConnectivityPlatformInterface
methods.
* Modify the MethodChannel implementation so it returns the right types.
* Bring utility methods, asserts and other logic that is only needed on
the MethodChannel implementation from the core package, so it's simpler.
* Bring MethodChannel unit tests from core package.

Co-authored-by: David Iglesias <ditman@gmail.com>
FlutterSu pushed a commit to FlutterSu/flutter-plugins that referenced this pull request Nov 20, 2020
…from the core package (flutter#2526)

* Bring ConnectivityResult and LocationAuthorizationStatus enums from
core package.
* Use the above Enums as return values for ConnectivityPlatformInterface
methods.
* Modify the MethodChannel implementation so it returns the right types.
* Bring utility methods, asserts and other logic that is only needed on
the MethodChannel implementation from the core package, so it's simpler.
* Bring MethodChannel unit tests from core package.

Co-authored-by: David Iglesias <ditman@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants