-
-
Notifications
You must be signed in to change notification settings - Fork 194
Livesync service refactoring #1941
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
Conversation
👍 |
if (installed) { | ||
platformLiveSyncService.afterInstallApplicationAction(deviceAppData, localToDevicePaths).wait(); | ||
|
||
if (device.applicationManager.canStartApplication()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can call:
device.applicationManager.tryStartApplication(appIdentifier).wait();
It does almost the same as your code :)
ping @rosen-vladimirov your feedback has been applied |
let installed = this.tryInstallApplication(device, deviceAppData).wait(); | ||
|
||
if(installed) { | ||
device.applicationManager.tryStartApplication(deviceAppData.appIdentifier).wait(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add an option here to call different action (in case when running debug + livesync). Perhaps it should be in platform-livesync-service-base to allow handling both ios and android.
7f2e693
to
3541c41
Compare
👍 |
forceExecuteFullSync: boolean; | ||
} | ||
|
||
interface IPlatformLiveSyncService { | ||
fullSync(postInstallAction?: (deviceAppData: Mobile.IDeviceAppData, localToDevicePaths: Mobile.ILocalToDevicePathData[]) => IFuture<void>): IFuture<void>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the action is used even when app is already installed, so the name is not really correct. postAction
seems better IMO, but it's just a suggestion, not a merge stopper :)
👍 great work |
…ons so that we could delete some platform specific ifs
3541c41
to
703e386
Compare
No description provided.