Skip to content

Reland fix --version#52141

Merged
dnfield merged 3 commits intoflutter:masterfrom
dnfield:fix_version
Mar 7, 2020
Merged

Reland fix --version#52141
dnfield merged 3 commits intoflutter:masterfrom
dnfield:fix_version

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Mar 6, 2020 •
edited
Loading

Relands #52062

This restricts the tag fetching to only flutter doctor or flutter commands where the user explicitly asks for the --version, so should not regress performance of other commands like flutter analyze or flutter run etc..

Fixes #15529 again

@dnfield dnfield requested review from jmagman and jonahwilliams March 6, 2020 22:03
@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 6, 2020
jonahwilliams
jonahwilliams approved these changes Mar 6, 2020
View reviewed changes
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

@jmagman jmagman mentioned this pull request Mar 7, 2020
Closed
@dnfield dnfield merged commit e13e170 into flutter:master Mar 7, 2020
@dnfield dnfield deleted the fix_version branch March 7, 2020 05:38
@zanderso
Copy link
Member

zanderso commented Mar 9, 2020

Is this going to affect the version reported in analytics and crash logging?

@dnfield
Copy link
Contributor Author

dnfield commented Mar 9, 2020

This only affects flutter doctor and flutter --version

@zanderso
Copy link
Member

zanderso commented Mar 9, 2020

Grepping for gitTagVersion, I see that commands/build_ios_framework.dart and commands/upgrade.dart mention it. I'm not sure about the former, but I can imagine needing to fetch tags before trying to upgrade.

@dnfield
Copy link
Contributor Author

dnfield commented Mar 9, 2020

build_ios_frameworks is using it to check if you're on dev/beta/stable - there's probably some situation where you could get messed up if your tags were really out of date, so it's probably worth fetching them there.

upgrade seems like it would benefit from this as well. I didn't really think of it because I don't really ever use that command.

@dnfield dnfield mentioned this pull request Mar 9, 2020
Closed
@dnfield
Copy link
Contributor Author

dnfield commented Mar 9, 2020

Opened #52250 to apply this logic to those two commands.

@tvolkert tvolkert mentioned this pull request Mar 26, 2020
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jmagman jmagman Awaiting requested review from jmagman

1 more reviewer

@jonahwilliams jonahwilliams jonahwilliams approved these changes

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.

For devs who git pull instead of flutter upgrade, version calculation is wrong

5 participants

@dnfield @zanderso @jonahwilliams @googlebot @fluttergithubbot