Dependency injection Attach command#113227
Conversation
flutter-dashboard
bot
added
the
tool
| required ProcessInfo processInfo, | ||
| required FileSystem fileSystem, | ||
| }): _artifacts = artifacts, | ||
| _hotRunnerFactory = (hotRunnerFactory == null) ? HotRunnerFactory() : hotRunnerFactory, |
There was a problem hiding this comment.
| _hotRunnerFactory = (hotRunnerFactory == null) ? HotRunnerFactory() : hotRunnerFactory, | |
| _hotRunnerFactory = hotRunnerFactory ?? HotRunnerFactory(), |
| } | ||
|
|
||
| globals.terminal.usesTerminalUi = daemon == null; | ||
| _terminal.usesTerminalUi = daemon == null; |
There was a problem hiding this comment.
Seems strange to me that we set this here; I would think that we would do it somewhere earlier. I'm just talking out loud though, keeping status quo LGTM for now.
christopherfujino
left a comment
christopherfujino
left a comment
There was a problem hiding this comment.
LGTM with a nit about using ?? over ternary
|
Thanks for migrating this command! |
|
|
||
| testUsingContext('exits when multiple devices connected', () async { | ||
| final AttachCommand command = AttachCommand(); | ||
| final AttachCommand command = AttachCommand( |
There was a problem hiding this comment.
Oh btw, are we still relying on all the overrides here? I would have hoped we could use testWithoutContext after this refactor.
There was a problem hiding this comment.
I just tried using testWithoutContext but it seems like the attach command and the BufferLogger used in this tests still needs a context
There was a problem hiding this comment.
Annoyingly FlutterCommand depends on globals :(
Jasguerrero
added
the
autosubmit
2 participants
Part of: #107607
Pre-launch Checklist
///).