-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix flutter analyze defaults when files are specified (#4091). #4348
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
Conversation
Disables current package and current directory analysis when files are specified. Fixes: flutter#4091.
|
This won't work because it would make us ignore the arguments if they're explicitly set. |
Agreed. I guess I was assuming that was not a common use case. Maybe I'm wrong though. Do you find yourself wanting to analyze a list of files and the current package or directory? I was seeing that as a degenerate case. Or? |
|
@Hixie : added a check to ensure that we only override settings if they are not user-specified. PTAL. |
|
I think I'd move the flutterRepo checks up into the top part as well, so that I agree that this is an edge case, but having this kind of polish throughout the entire project is what leads to people having confidence in the project. Everyone will hit some minor edge case. We have to get all of the edge cases right so that everyone's particular edge case works as they expect. |
|
In any case, LGTM once you're confident you've polished it enough that it shines ;-) |
👍 I totally agree here. Otherwise, I'm not totally sure what the desired behavior is WRT to
should analyze the repo, plus the directory, package and both respectively? Presumably, this should add
I'm inclined to tackle these variations with some tests in a follow-up PR after we clarify what variations we want to support (and how). Might be a good time to revisit the defaults too in the spirit of general polish. Sound reasonable? |
Totally. Maybe the right behavior is to have everything default off, and if you specify none, we use --current-package? |
|
Cool. Opened #4387 to track command cleanup. |
* Move texture registry ownership to platform view This enables the texture registry to survive activity pause on Android.
…" (flutter#4387) This reverts commit e58764f.
Disables current package and current directory analysis when files are specified.
Fixes: #4091.
cc @Hixie