Skip to content

[macos] Add BasicMessageChannel #162

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
merged 4 commits into from
Dec 5, 2018

Conversation

stuartmorgan-g
Copy link
Collaborator

Further API alignment (issue #102). Adds support for communicating with
a BasicMessageChannel on the Flutter side.

Also splits apart the files related to codecs and channels to be one class
per file. They were originally combined to match the Flutter iOS file structure,
but that has proven to hinder project navigation more than it helps with
cross-referencing.

Moved classes have been cleaned up to be more consistent about using
nullability declarations only on interfaces, not implementations.

Further API alignment (issue google#102). Adds support for communicating with
a BasicMessageChannel on the Flutter side.
@stuartmorgan-g
Copy link
Collaborator Author

The diff mostly did a good job with the file splitting, but one exception to be aware of is that MethodCall is just moved, not new code.

@@ -69,14 +69,14 @@ extern NSString const *_Nonnull FLEMethodNotImplemented;
*
* The arguments, if any, must be encodable using this channel's codec.
*/
- (void)invokeMethod:(nonnull NSString *)method arguments:(id _Nullable)arguments;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the qualifiers in FLEBasicMessageChannel.h change as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The nullability annotations on FLEBasicMessageChannel are using this style. The typedefs (as with those at the top of this file) can't, because this form isn't usable in all language constructions.

Copy link
Contributor

@franciscojma86 franciscojma86 left a comment

Choose a reason for hiding this comment

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

LGTM with one qualifiers question

@stuartmorgan-g stuartmorgan-g merged commit 5c75476 into google:master Dec 5, 2018
@stuartmorgan-g stuartmorgan-g deleted the mac-message-channel branch December 5, 2018 19:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 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.

2 participants