-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix version command for certain git workflows #52062
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
jmagman
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 with nit
| final String hash; | ||
|
|
||
| static GitTagVersion determine(ProcessUtils processUtils, [String workingDirectory]) { | ||
| _runGit('git fetch https://github.com/flutter/flutter --tags', processUtils, workingDirectory); |
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.
Would you mind making the repo URL a constant and adopting in fetchRemoteFrameworkCommitDate()?
| 'https://github.com/flutter/flutter.git', |
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.
Done
This reverts commit 20bf43b.
…r#52062)" (flutter#52128)" This reverts commit e7d70fb.
Description
Fixes #15529 by running
git fetch https://github.com/flutter/flutter --tagsbefore trying to use tags to describe the current version.If there is no network connection, the command just silently fails and we're no worse than we used to be. If there is a network connection, this will end up pulling down tags, and potentially updating the FETCH_HEAD.
Related Issues
Fixes #15529
additional context at #50577
Tests
I added the following tests:
Updated version_test.dart to handle the new invocation of git.
Added a test that asserts we fetch the tags when we try to determine the version.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.