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

Remove old platform messaging API #8837

Merged
merged 11 commits into from Mar 17, 2017

Conversation

mravn-google
Copy link
Contributor

@mravn-google mravn-google commented Mar 16, 2017

Breaking change: removed deprecated methods of PlatformMessages, leaving only binary messaging there. All other use of platform communication now goes through PlatformMessageChannel and PlatformMethodChannels. Retained use of String and JSON codecs for now.

Companion engine PR: flutter/engine#3482

/// An command object representing the invocation of a named method.
class MethodCall {
final String method;
final dynamic arguments;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put constructors first and doc these member variables.

}

@override
int get hashCode => method.hashCode ^ (arguments?.hashCode ?? 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use hashValues

Copy link
Contributor

Choose a reason for hiding this comment

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

also if possible put == and hashCode together so it's obvious that they're equivalent

import 'message_codecs.dart';

/// A JSON [PlatformMethodChannel] for navigation.
const PlatformMethodChannel flutterNavigationChannel = const PlatformMethodChannel(
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be a bit cleaner if there was a class that these channels all belonged to as static members. Perhaps:

SystemChannels.navigation
SystemChannels.platform

etc

@abarth
Copy link
Contributor

abarth commented Mar 16, 2017

LGTM


@override
bool operator== (dynamic o) {
if (o is! MethodCall)
Copy link
Contributor

Choose a reason for hiding this comment

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

this should probably be a runtimeType check so that == isn't sensitive to the order of its arguments

int get hashCode => method.hashCode ^ (arguments?.hashCode ?? 0);

@override
String toString() => 'MethodCall($method, $arguments)';
Copy link
Contributor

Choose a reason for hiding this comment

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

s/MethodCall/$runtimeType/ in case someone subclasses it

mravn-google added a commit to flutter/engine that referenced this pull request Mar 17, 2017
Breaking change: removed facilities for JSON and string messaging from FlutterView/FlutterViewController, leaving only binary messaging there. All other use of flutter communication now goes through FlutterMessageChannel and FlutterMethodChannels. Retained use of String and JSON codecs for now.

Companion flutter PR: flutter/flutter#8837
@mravn-google mravn-google merged commit dce4bf8 into flutter:master Mar 17, 2017
@mravn-google mravn-google deleted the remove_old_msg_api branch March 17, 2017 10:56
abarth added a commit that referenced this pull request Mar 17, 2017
brianosman pushed a commit to brianosman/engine that referenced this pull request Jun 16, 2017
Breaking change: removed facilities for JSON and string messaging from FlutterView/FlutterViewController, leaving only binary messaging there. All other use of flutter communication now goes through FlutterMessageChannel and FlutterMethodChannels. Retained use of String and JSON codecs for now.

Companion flutter PR: flutter/flutter#8837
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants