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

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Mar 26, 2020

Discovered when attempting to use this in google3. A number of method channels/platform channels call toString on an enum (e.g. Brightness.dark) to pass platform data back and forth.

It would be better if these just used indices (encoding a single int instead of encoding a string), we can't really stop customers from doing it and it can cause unexpected failures in release mode.

@jonahwilliams
Copy link
Contributor

Ahh yeah enums. I've definitely written some of this code myself, but it was easy to forget about it (apparently)

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@alexmarkov alexmarkov left a comment

Choose a reason for hiding this comment

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

lgtm

@dnfield dnfield added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Mar 26, 2020
@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Linux Fuchsia has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Mar 26, 2020
@dnfield
Copy link
Contributor Author

dnfield commented Mar 26, 2020

LUCI is red on an out of order commit - top of tree is actually green. Fuchsia builder on this is a flake, and this change does not impact fuchsia. Landing to kick tree.

@dnfield dnfield merged commit cda57c1 into flutter:master Mar 26, 2020
@dnfield dnfield deleted the enum_tostring branch March 26, 2020 20:12
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 27, 2020
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.

5 participants