Skip to content

Remove environment variable guards for command line desktop and web#33867

Merged
jonahwilliams merged 12 commits intoflutter:masterfrom
jonahwilliams:NO_MORE_ENVS
Jun 19, 2019
Merged

Remove environment variable guards for command line desktop and web#33867
jonahwilliams merged 12 commits intoflutter:masterfrom
jonahwilliams:NO_MORE_ENVS

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Jun 4, 2019 •
edited
Loading

Description

removes environment variables for command line for web and desktop, leaves them in for daemon

Fixes #30707

@jonahwilliams
Copy link
Contributor Author

jonahwilliams commented Jun 4, 2019

From offline feedback, we're going to remove the opt out

@goderbauer goderbauer added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 4, 2019
@jonahwilliams jonahwilliams changed the title (WIP) Make environment variables for web/desktop opt out for command line Make environment variables for web/desktop opt out for command line Jun 18, 2019
@jonahwilliams jonahwilliams marked this pull request as ready for review June 18, 2019 20:02
@jonahwilliams jonahwilliams changed the title Make environment variables for web/desktop opt out for command line Remove environment variable guards for command line desktop and web Jun 18, 2019
stuartmorgan-g
stuartmorgan-g reviewed Jun 19, 2019
View reviewed changes
dnfield
dnfield reviewed Jun 19, 2019
View reviewed changes
stuartmorgan-g
stuartmorgan-g approved these changes Jun 19, 2019
View reviewed changes
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g 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 a few more nits. The change to filter devices by project during run already landed in another PR I assume?

You could mark this as fixing 30707 since removing the flag was the last thing that was tracking.

@jonahwilliams
Copy link
Contributor Author

jonahwilliams commented Jun 19, 2019

The change to filter devices by project during run already landed in another PR I assume?

Yes, that landed a while back. Example: rm -rf flutter/examples/hello_world/web and then open a simulator/emulator attach a device. flutter run will choose the device over the web.

@jonahwilliams
Copy link
Contributor Author

jonahwilliams commented Jun 19, 2019

Actually, this needs one more change - when confronted with the choice between an ephemeral device and one that isn't, we should automagically choose the ephemeral one

@jonahwilliams
Copy link
Contributor Author

jonahwilliams commented Jun 19, 2019

Actually... I'm gonna walk that back a bit. We'll see if someone complains first

@jonahwilliams jonahwilliams merged commit 0d9a1b2 into flutter:master Jun 19, 2019
@jonahwilliams jonahwilliams deleted the NO_MORE_ENVS branch June 19, 2019 20:39
johnsonmh pushed a commit to johnsonmh/flutter that referenced this pull request Jul 30, 2019
@johnsonmh
Remove environment variable guards for command line desktop and web (f…
eda9cfd 
…lutter#33867)
@jamesderlin jamesderlin mentioned this pull request May 4, 2020
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

+1 more reviewer

@dnfield dnfield dnfield left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support 'flutter run' for desktop targets

5 participants

@jonahwilliams @stuartmorgan-g @dnfield @goderbauer @googlebot