Add coldboot option to AVD emulators#3381
Conversation
Hamdor
force-pushed
the
add_avd_coldboot_option
branch
from
ff3a965 to
231549f
Compare
DanTup
left a comment
DanTup
left a comment
There was a problem hiding this comment.
This is great, thanks! I've added some minor comments, but otherwise this looks good to me :-)
src/shared/vscode/device_manager.ts
Outdated
| } | ||
|
|
||
| type PickableDevice = vs.QuickPickItem & { device: f.Device | PlatformEnabler | Emulator | EmulatorCreator }; | ||
| type PickableDevice = vs.QuickPickItem & { device: f.Device | PlatformEnabler | Emulator | EmulatorCreator } & { coldBoot?: boolean | undefined | null }; |
There was a problem hiding this comment.
Do we need both undefined and null here? If so, it should be clear what each means (perhaps in a comment).
nits: I'd also use either ? or undefined but not both, and probably just include coldBoot in the same definition as the device field.
src/shared/vscode/device_manager.ts
Outdated
| coldBoot: true, | ||
| description: e.description, | ||
| device: e.device, | ||
| label: e.label + " (cold boot)", |
There was a problem hiding this comment.
Looking at the screenshot, I think it might look nicer if we put (cold boot) after android (I think in the description here?) rather than the label - what do you think?
There was a problem hiding this comment.
Yes, looks much better if set in the description field. I will change that.
DanTup
added
in flutter
Hamdor
force-pushed
the
add_avd_coldboot_option
branch
from
231549f to
5b31948
Compare
|
Setting the cold boot string in the description field looks as follows: |
src/shared/vscode/device_manager.ts
Outdated
| import { LogCategory } from "../enums"; | ||
| import * as f from "../flutter/daemon_interfaces"; | ||
| import { CustomEmulator, CustomEmulatorDefinition, Emulator, EmulatorCreator, IFlutterDaemon, Logger, PlatformEnabler } from "../interfaces"; | ||
| import { CustomEmulatorDefinition, Emulator, EmulatorCreator, IFlutterDaemon, Logger, PlatformEnabler } from "../interfaces"; |
There was a problem hiding this comment.
Strange, how did that change happen... I will fix that later today
DanTup
left a comment
DanTup
left a comment
There was a problem hiding this comment.
Looks good to me once the compile issue is fixed - thanks!
Hamdor
force-pushed
the
add_avd_coldboot_option
branch
from
5b31948 to
8eb032d
Compare
|
@DanTup , I have no idea why the one workflow failed. Maybe some random failure? Could you might hit rerun? |
It seems like a VS Code flake to me. There are a lot of moving parts in these tests so it's not unusual for random failures. The change all looks good to me - thanks very much for the contribution! :) |
2 participants
This pull request adds a cold boot option to each android based emulator.
The command palette looks as follows:
closes #3366