Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add coldboot option to AVD emulators #3381

Merged
merged 1 commit into from Jun 4, 2021

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

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

@@ -488,4 +503,4 @@ export class FlutterDeviceManager implements vs.Disposable {
}
}

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

Hamdor commented Jun 3, 2021

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

@@ -4,7 +4,7 @@ import { getAllProjectFolders } from "../../shared/vscode/utils";
import { cancelAction, runFlutterCreateDotAction, runFlutterCreateDotPrompt } from "../constants";
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
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