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

Allow enabling custom-devices feature on all channels by default #108787

Open
ardera opened this issue Aug 2, 2022 · 6 comments
Open

Allow enabling custom-devices feature on all channels by default #108787

ardera opened this issue Aug 2, 2022 · 6 comments
Labels
c: proposal A detailed proposal for a change to Flutter P3 Issues that are less important to the Flutter project team-tool Owned by Flutter Tool team tool Affects the "flutter" command-line tool. See also t: labels. triaged-tool Triaged by Flutter Tool team

Comments

@ardera
Copy link
Contributor

ardera commented Aug 2, 2022

Currently enabling the custom-devices feature will only take effect on master channel:

const Feature flutterCustomDevicesFeature = Feature(
name: 'Early support for custom device types',
configSetting: 'enable-custom-devices',
environmentOverride: 'FLUTTER_CUSTOM_DEVICES',
master: FeatureChannelSetting(
available: true,
),
);

While I can understand this is to make sure people don't forget it's not a "stable" feature, it's leading to a lot of problems for the users:

The users are often users of custom embedders, like ivi-homescreen, the sony embedder or flutter-pi. Actually toyota seems to be using it quite a lot. In a custom-embedder scenario, you're often using custom engine builds. And because building the flutter engine is a bit resource-intensive, most people are only doing it for stable:

So most people actually want to use custom-devices with the stable channel, which the SDK doesn't allow though. This requires weird workarounds, like going into the SDK dir and checking out the stable SDK commit directly, or checking out the remote branch (origin/stable).

Basically everytime I introduce the feature to someone I have to tell them to use the workaround. Toyota has a script lying around that does nothing else but patch the SDK sources to make it possible to enable the feature on stable.

I'd like to propose we change that and allow the feature to be enabled on any channel. If you're worried that people perceive it as a "stable" feature, we can add some messages like "(this feature is in alpha right now!)" when enabling it. Or we can just say it's stable now

@danagbemava-nc danagbemava-nc added in triage Presently being triaged by the triage team tool Affects the "flutter" command-line tool. See also t: labels. c: proposal A detailed proposal for a change to Flutter and removed in triage Presently being triaged by the triage team labels Aug 2, 2022
@jonahwilliams
Copy link
Member

This sounds good to me. I think we generally want to encourage folks to develop against stable anyway, so at this point the restriction may be do doing more harm than good. If @zanderso @christopherfujino are OK with it I think we should just remove the restriction

@zanderso
Copy link
Member

zanderso commented Aug 3, 2022

@ardera Would you be willing to expand on the documentation here https://github.com/flutter/flutter/wiki/Using-custom-embedders-with-the-Flutter-CLI as appropriate, and transfer to the website? Under the "Platform integration" docs probably makes sense. You could add a new .md file directly under https://github.com/flutter/website/tree/main/src/development/platform-integration.

@ardera
Copy link
Contributor Author

ardera commented Aug 8, 2022

@zanderso Yes sure, what expansions did you have in mind? Should I explain more how it works "under the hood", i.e. what commands there are, the available replacement values (${appName}, etc)? I guess once #90958 is merged some documentation for that would need to be added as well.

@zanderso
Copy link
Member

zanderso commented Aug 8, 2022

what commands there are, the available replacement values (${appName}, etc)? I guess once #90958 is merged some documentation for that would need to be added as well.

Yes, exactly.

@jwinarske
Copy link

Just enabling custom-devices on beta and stable is not enough to make it work. Changes from #108884 are also required. Otherwise flutter doctor will fail to find devices.

@zanderso
Copy link
Member

The necessary changes have been CP'd to the release branch. @ardera Would you like to leave this issue open to track adding the documentation?

@flutter-triage-bot flutter-triage-bot bot added the team-tool Owned by Flutter Tool team label Jul 8, 2023
@christopherfujino christopherfujino added P3 Issues that are less important to the Flutter project triaged-tool Triaged by Flutter Tool team labels Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: proposal A detailed proposal for a change to Flutter P3 Issues that are less important to the Flutter project team-tool Owned by Flutter Tool team tool Affects the "flutter" command-line tool. See also t: labels. triaged-tool Triaged by Flutter Tool team
Projects
None yet
Development

No branches or pull requests

6 participants