Repository navigation
[tool] Migrate CustomDevicesCommand to modular dependency injection - #190770
Conversation
2426710 to
c7415a7
Compare
c7415a7 to
b4a6f77
Compare
4feb1e7 to
a049bcf
Compare
There was a problem hiding this comment.
Code Review
This pull request refactors CustomDevicesCommand and its subcommands to accept and utilize ToolContext directly, making individual tool dependencies optional and simplifying command instantiation. Additionally, the associated hermetic tests are updated to use testWithoutContext instead of testUsingContext. Feedback on the changes suggests simplifying the instantiation of CustomDevicesCommand in the test helper by omitting redundant arguments that are already provided via toolContext.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the CustomDevicesCommand and its subcommands to accept a ToolContext parameter instead of multiple individual dependencies, such as FileSystem, Logger, Platform, ProcessManager, Terminal, and OperatingSystemUtils. This change simplifies the constructors and dependency injection across custom_devices.dart and executable.dart. Additionally, the associated unit tests in custom_devices_test.dart have been updated to use testWithoutContext instead of testUsingContext, and FakeTerminal has been simplified by extending AnsiTerminal. There are no review comments, and I have no feedback to provide.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the CustomDevicesCommand and its subcommands to accept a unified ToolContext instead of multiple individual dependencies, simplifying the constructor signatures. The test suite has also been updated to migrate several tests from testUsingContext to testWithoutContext and simplify FakeTerminal by extending AnsiTerminal. Feedback on the changes suggests migrating one remaining test case to testWithoutContext that was missed during the refactoring to maintain consistency.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors CustomDevicesCommand and its subcommands to accept ToolContext and retrieve dependencies directly from it, simplifying constructor signatures and removing the test factory. Correspondingly, hermetic tests in custom_devices_test.dart are migrated from testUsingContext to testWithoutContext, and FakeTerminal is updated to extend AnsiTerminal. The review feedback suggests using the Terminal interface instead of the concrete AnsiTerminal in test helper signatures to ensure flexibility.
Summary
Part 12 of the modular dependency injection migration.
Migrates
CustomDevicesCommandandCustomDevicesConfigto explicit constructor dependency injection (ToolContext), eliminating ambient global references and adding hermetic unit tests.Part of #47161