-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[flutter_conductor] validate git parsed version and release branch match #92064
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
| const String revision3 = '123abc'; | ||
| const String previousDartRevision = '171876a4e6cf56ee6da1f97d203926bd7afda7ef'; | ||
| const String nextDartRevision = 'f6c91128be6b77aef8351e1e3a9d07c85bc2e46e'; | ||
| const String previousVersion = '1.2.0-1.0.pre'; |
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.
This version was wrong previously, but nothing was validating it
| /// Will throw a [ConductorException] if the version is not possible given the | ||
| /// [candidateBranch] and [incrementLetter]. | ||
| void ensureValid(String candidateBranch, String incrementLetter) { | ||
| if (!const <String>{'x', 'y', 'z', 'm', 'n'}.contains(incrementLetter)) { |
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.
Is x a valid increment letter? I thought it was only m, n, y, z.
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.
good point
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.
updated without x
| 'branch $candidateBranch', | ||
| ); | ||
| } | ||
| } |
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.
Just curious, why we don't need to check n or z increment?
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.
incrementLetter is only here because if we're explicitly incrementing m we don't need to validate that the parsed m version matches the branch, since we are using the branch m.
|
LGTM |
Since the release version is a calculated based on the previous git tag for beta releases, this calculated version will be wrong if there was not a previous dev release. This adds a validating that the calculated version matches the candidate branch numbers.