-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[flutter_tools] do not validate maven upstream if cloud storage override provided #98293
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
[flutter_tools] do not validate maven upstream if cloud storage override provided #98293
Conversation
| ]; | ||
|
|
||
| /// MacOS specific required HTTP hosts. | ||
| const List<String> macOSRequiredHttpHosts = <String>[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the macOSRequiredHttpHosts changed to a method as well to keep it consistent with the android?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, since it would be a function that takes no parameters and always returns the same value. In the end, I guess it's a style choice.
Jasguerrero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ge override provided (flutter/flutter#98293)
…ge override provided (flutter/flutter#98293)
…ge override provided (flutter/flutter#98293)
…ge override provided (flutter/flutter#98293)
…ge override provided (flutter/flutter#98293)
…ge override provided (flutter/flutter#98293)
…ge override provided (flutter/flutter#98293)
| List<String> androidRequiredHttpHosts(Platform platform) { | ||
| return <String>[ | ||
| // If kEnvCloudUrl is set, it will be used as the maven host | ||
| if (!platform.environment.containsKey(kEnvCloudUrl)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check for kEnvCloudUrl in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh i see we always add that one
If a user provides the
FLUTTER_STORAGE_BASE_URLenv variable, this will be used as the maven host URL. However, the flutter http host doctor validator would still try to validate the upstream maven URL.This PR updates the validator to only check the upstream maven host if
FLUTTER_STORAGE_BASE_URLis NOT provided.Fixes #98170