Skip to content
This repository was archived by the owner on Feb 2, 2021. It is now read-only.

feat: Speed up device detection #1099

Merged
merged 1 commit into from
Jul 14, 2018

Conversation

rosen-vladimirov
Copy link
Collaborator

@rosen-vladimirov rosen-vladimirov commented Jul 2, 2018

Speed up device detection by:

  • removing duplicate calls to all device detection services
  • cache the Promise of initialize method - this way even when multiple calls are executed simultaneously, the result of the first one will be used. This allows Sidekick to call the method asynchronously.
  • do not execute iOS device detection in case --emulator is passed

Remove several public methods from the service - make them protected as we already have tests for them.
Remove all calls to detectCurrentlyAttachedDevices method outside of the service - the initialize method will do it anyway, so no need to call it externally.
Remove duplicate calls in the startLookingForDevices method - previously it had to filter which device detection services to call. As now each device detection service checks its platform, we can safely call all of them and rely on their filters to skip the execution.

@@ -1246,7 +1246,11 @@ interface IProfileDir {
profileDir: string;
}

interface ICommonOptions extends IRelease, IDeviceIdentifier, IJustLaunch, IAvd, IAvailableDevices, IProfileDir {
interface IEmulator {
Copy link
Contributor

Choose a reason for hiding this comment

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

IEmulator -> IHasEmulator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've moved the interface from NativeScript CLI. The idea of such interfaces is to wrap the properties used in $options (the ones that you can pass with -- on the terminal).In this case, the option the user can pass is --emulator, so the interface became IEmulator.

Ping @KristianDD , @Fatme - what do you think - should we change this to IHasEmulator (without changing the -- option.

Copy link
Contributor

Choose a reason for hiding this comment

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

IEmulatorTarget also sounds good to me

Copy link
Contributor

Choose a reason for hiding this comment

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

What I mean is that IEmulator should be an interface for something that wanna call itself an Emulator. Here we need an interface for something that just has an emulator property => we need something like IHasEmulator or IHasEmulatorOption/IOptionsWithEmulator if we wanna specify that these interfaces are wrapping some options values.

Speed up device detection by:
* removing duplicate calls to all device detection services
* cache the Promise of initialize method - this way even when multiple calls are executed simultaneously, the result of the first one will be used. This allows Sidekick to call the method asynchronously.
* do not execute iOS device detection login in case `--emulator` is passed

Remove several public methods from the service - make them protected as we already have tests for them.
Remove all calls to detectCurrentlyAttachedDevices method outside of the service - the initialize method will do it anyway, so no need to call it externally.
Remove duplicate calls in the startLookingForDevices method - previously it had to filter which device detection services to call. As now each device detection service checks its platform, we can safely call all of them and rely on their filters to skip the execution.
@rosen-vladimirov rosen-vladimirov force-pushed the vladimirov/improve-dev-detection branch from 98fa47a to 7cbf6c9 Compare July 13, 2018 10:46
@rosen-vladimirov rosen-vladimirov merged commit b8ec069 into master Jul 14, 2018
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/improve-dev-detection branch July 14, 2018 20:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants