Conversation
Eliminates ambient globals.dart usage from packages/flutter_tools/lib/src/artifacts.dart by injecting required dependencies into Artifacts.getLocalEngine, using package:path basename in LocalEngineInfo, and using a local FileSystem instance in _getFileGeneratorsPath.
There was a problem hiding this comment.
Code Review
This pull request refactors Artifacts.getLocalEngine to accept explicit dependencies instead of relying on global context, and updates FlutterCommandRunner and associated tests accordingly. Feedback on these changes includes correcting the Dart 3 pattern matching syntax in flutter_command_runner.dart to prevent compilation errors, instantiating LocalFileSystem locally within _getFileGeneratorsPath to avoid top-level side effects, and using platform-agnostic paths in the new unit test to ensure compatibility on Windows.
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
…ess review comments Accepts required ToolContext toolContext in Artifacts.getLocalEngine, moves localFileSystem inside _getFileGeneratorsPath, and uses platform-agnostic paths in artifacts_test.dart.
There was a problem hiding this comment.
Code Review
This pull request refactors Artifacts.getLocalEngine and related components to accept and use ToolContext directly instead of relying on global variables from globals.dart. It also updates FlutterCommandRunner to destructure _toolContext and pass it down, and updates associated tests to use testWithoutContext with FakeToolContext. Feedback suggests extracting the instantiation of LocalFileSystem in _getFileGeneratorsPath to a top-level private variable to avoid recreating it on every invocation.
Migrates Artifacts and CachedArtifacts away from globals.dart to explicit dependency injection via ToolContext.
Part of #188471
Pre-launch Checklist
///).