Repository navigation
[flutter_tools] refactor local engine locator to its own class - #67274
Conversation
flutter-dashboard
Bot
added
tool
This comment has been minimized.
This comment has been minimized.
jonahwilliams
changed the title
| // file://flutter-engine-local-path/src/out/host_debug_unopt/gen/dart-pkg/sky_engine/lib/ | ||
| if (engineUri?.path != null) { | ||
| engineSourcePath = _fileSystem.directory(engineUri.path) | ||
| ?.parent |
There was a problem hiding this comment.
nit: I would have expected the question mark to come at the end of the last line rather than the beginning of this one, but if this style is consistent with other uses it should be fine.
There was a problem hiding this comment.
?. is one operator though, splitting them up seems odd - or do you mean leaving both on a previous line?
There was a problem hiding this comment.
Hm. Dartfmt does indeed move the ? part to the next line as well. Maybe I'm just starting to think in terms of null-safety world, with ? as one operator and . as another, but this is fine.
| xcode: globals.xcode, | ||
| platform: globals.platform, | ||
| ), | ||
| LocalEngineLocator: () => LocalEngineLocator( |
There was a problem hiding this comment.
I thought we were trying to avoid adding new globals to the context and instead providing ctor initialization. Is that not possible for some reason in this case, or are we not trying to do that anymore?
There was a problem hiding this comment.
The initialization of the flutter commands still needs some work, but eventually yet.
There was a problem hiding this comment.
err "eventually yes"
There was a problem hiding this comment.
Can we just introduce this into the ctors of the necessary commands in this patch rather than introducing more work to refactor?
There was a problem hiding this comment.
I considered that but it is not feasible until #67275 and others have been completed and initialization of the tool is moved out of the flutter command
|
Thanks Dan! |
3 participants
Description
Make the logic for locating a local engine path part of its own class, add documentation, and update tests to cover engine source path locating too.
#47161