Skip to content

Conversation

@Hamdor
Copy link
Contributor

@Hamdor Hamdor commented Jun 3, 2021

This pull request adds a cold boot option to each android based emulator.

The command palette looks as follows:
launch-emulator

closes #3366

@Hamdor Hamdor force-pushed the add_avd_coldboot_option branch from ff3a965 to 231549f Compare June 3, 2021 17:32
Copy link
Member

@DanTup DanTup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks! I've added some minor comments, but otherwise this looks good to me :-)

}

type PickableDevice = vs.QuickPickItem & { device: f.Device | PlatformEnabler | Emulator | EmulatorCreator };
type PickableDevice = vs.QuickPickItem & { device: f.Device | PlatformEnabler | Emulator | EmulatorCreator } & { coldBoot?: boolean | undefined | null };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

coldBoot: true,
description: e.description,
device: e.device,
label: e.label + " (cold boot)",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, looks much better if set in the description field. I will change that.

@DanTup DanTup added this to the v3.24.0 milestone Jun 3, 2021
@DanTup DanTup added in flutter Relates to running Flutter apps is enhancement labels Jun 3, 2021
@Hamdor Hamdor force-pushed the add_avd_coldboot_option branch from 231549f to 5b31948 Compare June 3, 2021 20:19
@Hamdor
Copy link
Contributor Author

Hamdor commented Jun 3, 2021

Setting the cold boot string in the description field looks as follows:
launch-emulator

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";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, how did that change happen... I will fix that later today

Copy link
Member

@DanTup DanTup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me once the compile issue is fixed - thanks!

@Hamdor Hamdor force-pushed the add_avd_coldboot_option branch from 5b31948 to 8eb032d Compare June 4, 2021 14:29
@Hamdor
Copy link
Contributor Author

Hamdor commented Jun 4, 2021

@DanTup , I have no idea why the one workflow failed. Maybe some random failure? Could you might hit rerun?

@DanTup
Copy link
Member

DanTup commented Jun 4, 2021

[main 2021-06-04T14:48:07.896Z] CodeWindow: failed to load workbench window: 

Exit code:   1

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! :)

@DanTup DanTup merged commit 3af8a9c into Dart-Code:master Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in flutter Relates to running Flutter apps is enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(Flutter) Allow cold boot launch using vs code command

2 participants