-
Notifications
You must be signed in to change notification settings - Fork 241
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
fix: Fix FormatException caused by '@' in Custom Flutter Version #808
base: main
Are you sure you want to change the base?
Conversation
@franticn is attempting to deploy a commit to the FlutterTools Team on Vercel. A member of the Team first needs to authorize it. |
{ "date": "2025-01-13T23:01:03.050Z", "files": [ { "name": "releases_macos.json", "deltaBytes": 790, "source": "https://storage.googleapis.com/flutter_infra_release/releases/releases_macos.json" }, { "name": "releases_windows.json", "deltaBytes": 395, "source": "https://storage.googleapis.com/flutter_infra_release/releases/releases_windows.json" }, { "name": "releases_linux.json", "deltaBytes": 394, "source": "https://storage.googleapis.com/flutter_infra_release/releases/releases_linux.json" } ] }
{ "date": "2025-01-21T21:01:00.965Z", "files": [ { "name": "releases_macos.json", "deltaBytes": 790, "source": "https://storage.googleapis.com/flutter_infra_release/releases/releases_macos.json" }, { "name": "releases_windows.json", "deltaBytes": 395, "source": "https://storage.googleapis.com/flutter_infra_release/releases/releases_windows.json" }, { "name": "releases_linux.json", "deltaBytes": 394, "source": "https://storage.googleapis.com/flutter_infra_release/releases/releases_linux.json" } ] }
{ "date": "2025-01-27T21:00:59.751Z", "files": [ { "name": "releases_macos.json", "deltaBytes": 860, "source": "https://storage.googleapis.com/flutter_infra_release/releases/releases_macos.json" }, { "name": "releases_windows.json", "deltaBytes": 430, "source": "https://storage.googleapis.com/flutter_infra_release/releases/releases_windows.json" }, { "name": "releases_linux.json", "deltaBytes": 429, "source": "https://storage.googleapis.com/flutter_infra_release/releases/releases_linux.json" } ] }
WalkthroughThe pull request introduces updates to Flutter version management across multiple platforms. The changes primarily involve modifying the Changes
Sequence DiagramsequenceDiagram
participant Parser as FlutterVersion.parse()
participant Version as FlutterVersion
alt Custom Version
Parser ->> Parser: Check for 'custom_' prefix
Parser ->> Version: Create custom version
else Standard Version
Parser ->> Parser: Split version string
Parser ->> Version: Create standard version
end
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/src/models/flutter_version_model.dart (1)
61-64
: LGTM! Consider adding documentation for custom version handling.The change correctly fixes the FormatException by checking for custom versions before splitting on '@'. This ensures custom versions like 'custom_3.7.0@huawei' are handled properly.
Consider adding a code comment explaining that custom versions are handled as-is without parsing to prevent '@' symbol issues:
factory FlutterVersion.parse(String version) { - // Check if its custom. + // Handle custom versions first and preserve their full name without parsing + // to support version strings containing '@' (e.g., custom_3.7.0@huawei) if (version.startsWith('custom_')) { return FlutterVersion.custom(version); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/src/models/flutter_version_model.dart
(1 hunks)releases_linux.json
(1 hunks)releases_macos.json
(2 hunks)releases_windows.json
(1 hunks)test/models/flutter_version_model_test.dart
(3 hunks)
🔇 Additional comments (10)
test/models/flutter_version_model_test.dart (3)
83-88
: LGTM! Good test coverage for custom constructor.The test verifies that the custom constructor correctly handles version strings containing '@' symbols.
119-124
: LGTM! Good test coverage for parse method.The test verifies that the parse method correctly identifies and preserves custom version strings containing '@' symbols.
179-182
: LGTM! Good test coverage for isCustom getter.The test verifies that isCustom correctly identifies custom versions containing '@' symbols.
releases_linux.json (3)
15-15
: Verify the release dates in the new entriesThe release dates for both new entries are set in 2025:
- Line 15: "2025-01-27T20:27:45.101671Z"
- Line 25: "2025-01-21T20:35:29.161615Z"
These future dates seem incorrect and should be reviewed.
Also applies to: 25-25
9-18
: LGTM! Release entries follow the standard formatThe new release entries contain all required fields and follow the consistent format:
- hash
- channel
- version
- dart_sdk_version
- dart_sdk_arch
- release_date
- archive
- sha256
Also applies to: 19-28
4-4
: LGTM! Current release hashes updated correctlyThe current_release section is properly updated with new hashes pointing to the latest releases for beta and stable channels.
Also applies to: 6-6
releases_windows.json (2)
4-6
: LGTM! Current release updates look good.The updates to current_release section are consistent with the latest beta and stable versions.
9-38
: LGTM! New release entries are properly formatted.The new release entries for versions 3.29.0-0.2.pre, 3.27.3, and 3.27.2 contain all required fields and follow the established schema.
Note: While these changes appear correct, they seem unrelated to the PR's objective of fixing FormatException for custom Flutter versions with '@' symbols. Could you clarify how these version updates help address the reported issue?
releases_macos.json (2)
4-4
: Version references in current_release are consistentThe current_release section correctly references the latest beta (3.29.0-0.2.pre) and stable (3.27.3) versions that match with their corresponding release entries.
Also applies to: 6-6
314-314
: Formatting improvements in existing entriesThe changes to existing release entries improve readability through consistent indentation and line breaks.
Also applies to: 316-317, 324-324, 326-327
"version": "3.29.0-0.2.pre", | ||
"dart_sdk_version": "3.7.0 (build 3.7.0-323.1.beta)", | ||
"dart_sdk_arch": "x64", | ||
"release_date": "2025-01-27T20:19:00.875075Z", |
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.
Release dates are set in the future
The release dates for new entries are set in 2025, which is beyond the current date. This could cause issues with release management and version comparisons.
Update the release dates to use the correct current or past dates. For example:
- "release_date": "2025-01-27T20:19:00.875075Z",
+ "release_date": "2024-01-27T20:19:00.875075Z"
Also applies to: 25-25, 35-35, 45-45, 55-55, 65-65
As described in the documentation on https://fvm.app/documentation/advanced/custom-version, when we want to manage a custom version of the Flutter SDK in FVM, we need to use
custom_
as a prefix for the version. However, it does not support naming conventions that include an@
symbol, such ascustom_3.7.0@huawei
. This is because previously, theFlutterVersion#parse(String version)
method would first attempt to parse aFlutterVersion.release
type ofFlutterVersion
, and would split the string at the@
symbol. This would cause parsing errors for custom version numbers likecustom_3.7.0@huawei
. Therefore, I have prioritized the parsing ofFlutterVersion.custom
to ensure that suchFlutterVersion
can be successfully parsed.Summary by CodeRabbit
New Features
Bug Fixes
Documentation