Add --force to roll_dev.dart#56501
Conversation
| runGit('push $origin HEAD:dev', 'land the new version on the "dev" branch'); | ||
| git.run('push $origin $version', 'publish the version'); | ||
| git.run( | ||
| 'push ${force ? "--force " : ""}$origin HEAD:dev', |
There was a problem hiding this comment.
This is change 1
| // describe the latest dev release | ||
| const String ref = 'refs/heads/dev'; | ||
| return git.getOutput( | ||
| 'describe --match $glob --exact-match --tags $ref', |
There was a problem hiding this comment.
This is change 2
| // of the form: x.y.z-m.n.pre-c-g<revision> | ||
| final RegExp versionPattern = RegExp( | ||
| r'^(\d+)\.(\d+)\.(\d+)-(\d+)\.(\d+)\.pre-(\d+)-g([a-f0-9]+)$'); | ||
| r'^(\d+)\.(\d+)\.(\d+)-(\d+)\.(\d+)\.pre$'); |
There was a problem hiding this comment.
This is part of change 2. Instead of describing HEAD, we grab the exact tag at the tip of refs/heads/dev.
| _reportGitFailureAndExit(result, explanation); | ||
| return null; // for the analyzer's sake | ||
| } | ||
| class Git { |
There was a problem hiding this comment.
Created this wrapper so we can inject mocks for tests.
| // of the form: x.y.z-m.n.pre-c-g<revision> | ||
| final RegExp versionPattern = RegExp( | ||
| r'^(\d+)\.(\d+)\.(\d+)-(\d+)\.(\d+)\.pre-(\d+)-g([a-f0-9]+)$'); | ||
| r'^(\d+)\.(\d+)\.(\d+)-(\d+)\.(\d+)\.pre$'); |
There was a problem hiding this comment.
Does the comment need to updated too?
There was a problem hiding this comment.
Good catch
| } | ||
| class Git { | ||
| const Git(); | ||
| String getOutput(String command, String explanation) { |
There was a problem hiding this comment.
| String getOutput(String command, String explanation) { | |
| String getOutput(String command, String explanation) { |
dev/tools/test/roll_dev_test.dart
Outdated
| } | ||
| const String pattern = r'The current directory is not a Flutter ' | ||
| 'repository checkout with a correctly configured upstream remote.'; | ||
| expect(exception.toString(), contains(pattern)); |
There was a problem hiding this comment.
If this fails to catch something, this will throw an NPE here instead of failing to match.
There was a problem hiding this comment.
Apparently null implements .toString()
There was a problem hiding this comment.
I guess it's still safer to use exception?.toString() though, so it doesn't match 'null'...
dev/tools/test/roll_dev_test.dart
Outdated
| const String pattern = r'Your git repository is not clean. Try running ' | ||
| '"git clean -fd". Warning, this will delete files! Run with -n to find ' | ||
| 'out which ones.'; | ||
| expect(exception.toString(), contains(pattern)); |
jonahwilliams
left a comment
jonahwilliams
left a comment
There was a problem hiding this comment.
LGTM with nits. I would make sure the comments describing the tag format are all up to date too
4 participants
Description
This PR does the following:
--forceflag, since dev releases can have cherry picks.Related Issues
FIxes #56340
Tests
Added extensive unit tests, exercising existing changed logic and the pre-existing logic.