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

Conversation

@robert-ancell
Copy link
Contributor

The Linux embedder had a number of handlers for Flutter channels that were named plugin/handler/manager. Rename these all to handler to be consistent and reduce confusion with Flutter plugins (which these don't use the infrastructure for).

Handler seemed to be the more common term looking at other embedders like Windows, but even there it's not used consistently. Open to other terms.

This matches the naming used in the other embedders and removes
the confusion between this and Flutter plugins.
This matches the naming used in the other embedders and removes
the confusion between this and Flutter plugins.
This matches the naming used in the other embedders and removes
the confusion between this and Flutter plugins.
Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

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

LGTM!

Open to other terms.

I agree plugin is confusing. I don't have a strong preference between handler/manager, "handler" seems like a good thing for us to converge on :)

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

LGTM stamp from a Japanese personal seal

Thanks! This reads a lot better. We should consider similar renamings in other embedders as well.

Seems like the KeyboardHandler tests are upset after this.

@robert-ancell
Copy link
Contributor Author

Still looking into the test failure. It's an ASAN issue, i.e. a leak.

@jason-simmons
Copy link
Member

For the LSAN issue: this line in the suppression file needs to be updated to fl_keyboard_handler_test.cc:

https://github.com/flutter/engine/blob/main/testing/lsan_suppressions.txt#L54

@robert-ancell robert-ancell merged commit e431738 into flutter:main Jul 19, 2024
@robert-ancell robert-ancell deleted the linux-rename-plugin-to-handler branch July 19, 2024 00:44
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 19, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants