Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Add platform interface #34

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

Merged

Conversation

@JeroenWeener
Copy link
Contributor

@JeroenWeener JeroenWeener commented Mar 8, 2023

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

This PR adds a platform interface for the plugin. This interface is part of the ongoing effort to refactor the code to use the federated plugin architecture. The GooglePlayServicesAvailability model has been copied over, and its original version will be removed in a later PR.

⤵️ What is the current behavior?

The plugin uses a non-federated architecture.

🆕 What is the new behavior (if this is a feature change)?

The plugin will function the same. The platform interface code is contained in the new directory /google-api-availability-platform-interface and is not used outside of it.

💥 Does this PR introduce a breaking change?

No.

🐛 Recommendations for testing

This PR adds tests for the newly added GoogleApiAvailabilityPlatform. These tests can be found in google-api-availability-platform-interface/test/google_api_availability_platform_interface_test.dart.

📝 Links to relevant issues/docs

This PR is part of the ongoing effort to refactor this plugin to use the federated architecture requested in #33.

🤔 Checklist before submitting

  • All projects build
  • Follows style guide lines (code style guide)
  • Relevant documentation was updated
  • Rebased onto current develop

@@ -0,0 +1,3 @@
## 1.0.0

- Initial release.
Copy link
Member

@mvanbeusekom mvanbeusekom Mar 8, 2023

Choose a reason for hiding this comment

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

Can we put something more meaning full here (and following the Google Style guide)? Maybe something a long the lines of:

Suggested change
- Initial release.
- Extracts the common platform interface from the google_api_availability package.

JeroenWeener reacted with thumbs up emoji
Comment on lines 20 to 34
static GoogleApiAvailabilityPlatform _instance =
MethodChannelGoogleApiAvailability();

/// The default instance of [GoogleApiAvailabilityPlatform] to use.
///
/// Defaults to [MethodChannelGoogleApiAvailability].
static GoogleApiAvailabilityPlatform get instance => _instance;

/// Platform-specific plugins should set this with their own platform-specific
/// class that extends [GoogleApiAvailabilityPlatform] when they register
/// themselves.
static set instance(GoogleApiAvailabilityPlatform instance) {
PlatformInterface.verify(instance, _token);
_instance = instance;
}
Copy link
Member

@mvanbeusekom mvanbeusekom Mar 8, 2023

Choose a reason for hiding this comment

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

The use of a default implementation is no longer recommended as it increases maintenance for something that most likely will never be used.

Current recommended solution is to make the _instance field nullable. Something like this:

Suggested change
static GoogleApiAvailabilityPlatform _instance =
MethodChannelGoogleApiAvailability();
/// The default instance of [GoogleApiAvailabilityPlatform] to use.
///
/// Defaults to [MethodChannelGoogleApiAvailability].
static GoogleApiAvailabilityPlatform get instance => _instance;
/// Platform-specific plugins should set this with their own platform-specific
/// class that extends [GoogleApiAvailabilityPlatform] when they register
/// themselves.
static set instance(GoogleApiAvailabilityPlatform instance) {
PlatformInterface.verify(instance, _token);
_instance = instance;
}
static GoogleApiAvailabilityPlatform? _instance;
/// The default instance of [GoogleApiAvailabilityPlatform] to use.
///
/// Defaults to [MethodChannelGoogleApiAvailability].
static GoogleApiAvailabilityPlatform? get instance => _instance;
/// Platform-specific plugins should set this with their own platform-specific
/// class that extends [GoogleApiAvailabilityPlatform] when they register
/// themselves.
static set instance(GoogleApiAvailabilityPlatform? instance) {
if (instance == null) {
throw AssertionError(
'Platform interfaces can only be set to a non-null instance');
}
PlatformInterface.verify(instance, _token);
_instance = instance;
}

JeroenWeener reacted with thumbs up emoji
import '../models/google_play_services_availability.dart';

/// An implementation of [GoogleApiAvailabilityPlatform] that uses method channels.
class MethodChannelGoogleApiAvailability extends GoogleApiAvailabilityPlatform {
Copy link
Member

@mvanbeusekom mvanbeusekom Mar 8, 2023

Choose a reason for hiding this comment

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

Adding a default implementation is no longer recommended, meaning this file should be deleted. See also comments in the google_api_availability_platform_interface.dart file.

JeroenWeener reacted with thumbs up emoji
Copy link
Member

@mvanbeusekom mvanbeusekom left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mvanbeusekom mvanbeusekom mvanbeusekom approved these changes

@paulppn paulppn Awaiting requested review from paulppn

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /