feat: support --flutter-version=fvm and --flutter-version=system #3238
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
packages/shorebird_cli/lib/src/commands/release/release_command.dart
Outdated
Show resolved
Hide resolved
| /// Engine • revision 8cd19e509d (5 weeks ago) • 2025-06-12 16:30:12 -0700 | ||
| /// Tools • Dart 3.8.1 • DevTools 2.45.1 | ||
| /// ``` | ||
| String? maybeParseFlutterVersionFromOutput(String output) { |
There was a problem hiding this comment.
I still think we should consolidate this with ShorebirdFlutter. The intention of ShorebirdFlutter was to interface with the underlying Flutter installation. If we merged this we would have flutter --version execution and parsing in multiple places.
| /// Returns the Flutter version from a FVM-installed Flutter if available. | ||
| String flutterVersionFromFVMFlutter(ShorebirdProcess process) { | ||
| try { | ||
| final output = process.runSync('fvm', ['flutter', '--version']); |
There was a problem hiding this comment.
What if fvm isn't available on the PATH? I can imagine cases where users create aliases for flutter which proxy to the underlying fvm installation behind the scenes 🤷
See also:
Tighter VSCode integration, with configuration and settings management. FVM will now automatically configure VSCode to use the correct Flutter SDK version, triggering a terminal path update, so you can just use flutter, commands instead of fvm flutter
| import 'package:pub_semver/pub_semver.dart'; | ||
| import 'package:scoped_deps/scoped_deps.dart'; | ||
| import 'package:shorebird_cli/src/executables/executables.dart'; | ||
| import 'package:shorebird_cli/src/executables/flutter_version.dart'; |
There was a problem hiding this comment.
This import shouldn't be needed if we export flutter_version from executables/executables.dart
Also the convention would be to create a class called Flutter under executables which exposes whatever we need from the underlying flutter tool. This is what the ShorebirdFlutter class was meant to do.
| late ShorebirdEnv shorebirdEnv; | ||
| late ShorebirdFlutter shorebirdFlutter; | ||
| late ShorebirdValidator shorebirdValidator; | ||
| late ShorebirdProcess process; // Only used by resolveTargetFlutterRevision. |
There was a problem hiding this comment.
Ideally we should just mock the direct dependency (e.g. ShorebirdFlutter or Flutter) and not have to mock a transitive dependency here.
I chose this approach so we didn't have to write any code to read .fvmrc files, and all we do is just know to run
fvm flutterrather thanflutterwhen determining the version from the system-available flutter.Fixes #1385
And is part of potentially fixing #2529.