Clean up build window #6502

Troy-Ferrell · 2019-11-06T22:12:11Z

Overview

Build window inspector changes. This is a first phase of review for Build window and bring up of the feature area. This change is primarily eliminating key error areas and simplifying the code to be more manageable, readable, and easier to navigate. Additional issues are tracked in #6520

General refactoring to break up monolithic functions into sub-components
General refactoring to simplify redundant code into re-usable functions
Cleaned up UI

Key areas:

Build directory selection is now selected via FolderPanel utility instead of just a string (still broken to only point relative to project)
Deploy tab had some weird code handling for if a target was 127.0.0.1 and how it updated machine name. Many of the assumptions in the code have been removed
Simplified access of CurrentConnection and CurrentConnectionIndex, moving guards to these properties instead of scattered throughout code
Added additional GUIContent constant labels
Parallelized tasks for installing/uninstalling appx instead of waiting for each one

BEFORE

AFTER

New Deploy view

Changes

Fixes: # .

Verification

As a reviewer, it is possible to check out this change locally by using the following
commands (substituting {PR_ID} with the ID of this pull request):

git fetch origin pull/{PR_ID}/head:name_of_local_branch

git checkout name_of_local_branch

Troy-Ferrell · 2019-11-06T22:12:48Z

@keveleigh FYI, I saw you have some PRs out on Build window. Once in, I'll handle merging them in with this. My changes are mostly in the inspector

…ovements # Conflicts: # Assets/MixedRealityToolkit/Inspectors/Setup/BuildDeployWindow.cs

keveleigh

Would be great if, when this comes out of Draft, it's split up into a few PRs.
Especially BuildDeployWindow.cs is hard to review with the sheer amount of things moved around and reworked.

Assets/MixedRealityToolkit/Inspectors/Setup/BuildDeployWindow.cs

…ovements

Troy-Ferrell · 2019-11-11T21:33:46Z

@keveleigh I'm still doing some final tests...and there are still all the issues in #6520 but the PR is close enough for review now!

Troy-Ferrell · 2019-11-11T21:33:53Z

/azp run

azure-pipelines · 2019-11-11T21:34:06Z

Azure Pipelines successfully started running 2 pipeline(s).

keveleigh · 2019-12-12T00:20:07Z

Looks like some merge conflicts here

…ovements # Conflicts: # Assets/MixedRealityToolkit/Utilities/BuildAndDeploy/BuildDeployWindow.cs # Assets/MixedRealityToolkit/Utilities/WindowsDevicePortal/DevicePortal.cs

Troy-Ferrell · 2019-12-12T20:34:43Z

@keveleigh resolved

Troy-Ferrell · 2019-12-12T20:35:39Z

/azp run

azure-pipelines · 2019-12-12T20:35:53Z

Azure Pipelines successfully started running 2 pipeline(s).

keveleigh

Some comments from first pass. Will pull this down and test locally now.

Also, will need a merge with #6903 and ARM64 support.

Assets/MixedRealityToolkit/Inspectors/Setup/BuildDeployWindow.cs

Assets/MixedRealityToolkit/Utilities/BuildAndDeploy/BuildDeployWindow.cs

Assets/MixedRealityToolkit/Inspectors/Setup/BuildDeployWindow.cs

Assets/MixedRealityToolkit/Utilities/BuildAndDeploy/BuildDeployWindow.cs

Troy-Ferrell · 2019-12-16T19:42:26Z

Also need to investigate testconnection issue:

EDIT
Believe it may be issue in DeviceInfo.Authorization databag not updating

EDIT 2
It's actually because of UseSLL on and only fails when targeting remote

EDIT 3
Believe this is dupe of #6877

…ovements # Conflicts: # Assets/MixedRealityToolkit/Utilities/BuildAndDeploy/BuildDeployWindow.cs

Assets/MixedRealityToolkit/Inspectors/Setup/BuildDeployWindow.cs

Troy-Ferrell · 2019-12-16T23:06:26Z

/azp run

azure-pipelines · 2019-12-16T23:06:43Z

Azure Pipelines successfully started running 3 pipeline(s).

Troy-Ferrell · 2019-12-16T23:17:46Z

UninstallApp and other components need to be updated

keveleigh · 2019-12-17T00:08:49Z

/azp run

azure-pipelines · 2019-12-17T00:09:05Z

Azure Pipelines successfully started running 2 pipeline(s).

keveleigh · 2019-12-17T00:10:23Z

/azp run

azure-pipelines · 2019-12-17T00:10:37Z

Azure Pipelines successfully started running 2 pipeline(s).

Troy-Ferrell · 2019-12-17T00:45:14Z

/azp run

azure-pipelines · 2019-12-17T00:45:29Z

Azure Pipelines successfully started running 2 pipeline(s).

Troy-Ferrell · 2019-12-17T01:52:19Z

Weird bug where CI pipelines are not reporting in PR issue. They both passed though, see links below

https://dev.azure.com/aipmr/MixedRealityToolkit-Unity-CI/_build/results?buildId=7891&view=results
https://dev.azure.com/aipmr/MixedRealityToolkit-Unity-CI/_build/results?buildId=7890&view=results

Merging pull request...

Build window cleanup

f87655d

Troy-Ferrell self-assigned this Nov 6, 2019

Troy-Ferrell added the Build Window label Nov 6, 2019

Troy-Ferrell added 3 commits November 6, 2019 14:45

More updates

102a1f2

Merge branch 'mrtk_development' into users/trferrel/build-window-impr…

70fef01

…ovements # Conflicts: # Assets/MixedRealityToolkit/Inspectors/Setup/BuildDeployWindow.cs

More fixes

cac3281

keveleigh reviewed Nov 7, 2019

View reviewed changes

Assets/MixedRealityToolkit/Inspectors/Setup/BuildDeployWindow.cs Outdated Show resolved Hide resolved

Assets/MixedRealityToolkit/Inspectors/Setup/BuildDeployWindow.cs Show resolved Hide resolved

Troy-Ferrell added 4 commits November 7, 2019 15:54

Cleaning up code

6715568

revert from master

a6a29da

proofread of code

66fe62a

Merge branch 'mrtk_development' into users/trferrel/build-window-impr…

ae4ee5e

…ovements

Troy-Ferrell marked this pull request as ready for review November 11, 2019 21:32

Troy-Ferrell requested review from david-c-kline and wiwei as code owners November 11, 2019 21:32

Troy-Ferrell requested a review from keveleigh November 11, 2019 21:33

Troy-Ferrell mentioned this pull request Nov 18, 2019

Build Window improvements 2 #6617

Merged

Merge branch 'mrtk_development' into users/trferrel/build-window-impr…

4d47995

…ovements # Conflicts: # Assets/MixedRealityToolkit/Utilities/BuildAndDeploy/BuildDeployWindow.cs # Assets/MixedRealityToolkit/Utilities/WindowsDevicePortal/DevicePortal.cs

microsoft deleted a comment from azure-pipelines bot Dec 12, 2019

keveleigh reviewed Dec 16, 2019

View reviewed changes

Assets/MixedRealityToolkit/Inspectors/Setup/BuildDeployWindow.cs Outdated Show resolved Hide resolved

Assets/MixedRealityToolkit/Utilities/BuildAndDeploy/BuildDeployWindow.cs Show resolved Hide resolved

Troy-Ferrell added 2 commits December 16, 2019 12:10

Fixes for PR

ceed975

Merge branch 'mrtk_development' into users/trferrel/build-window-impr…

58b51a0

…ovements # Conflicts: # Assets/MixedRealityToolkit/Utilities/BuildAndDeploy/BuildDeployWindow.cs

keveleigh reviewed Dec 16, 2019

View reviewed changes

Assets/MixedRealityToolkit/Inspectors/Setup/BuildDeployWindow.cs Show resolved Hide resolved

Troy-Ferrell added 6 commits December 16, 2019 13:18

use ssl bug

a06a142

Updates for PR

c7e4b22

Update machine name info display

a1bb9b3

Clean up local connection retrieval

65965dc

Update index tracking

9a80369

Move build window to MRTK.tools

f39622f

Troy-Ferrell requested a review from MenelvagorMilsom as a code owner December 16, 2019 23:02

Troy-Ferrell requested a review from keveleigh December 16, 2019 23:06

Clean up action functions for targeted devices

309aa03

keveleigh approved these changes Dec 17, 2019

View reviewed changes

Fix mismatch in values

f0e6af0

Troy-Ferrell merged commit 9091059 into microsoft:mrtk_development Dec 17, 2019

Troy-Ferrell deleted the users/trferrel/build-window-improvements branch January 22, 2020 19:02

Clean up build window #6502

Clean up build window #6502

Uh oh!

Conversation

Troy-Ferrell commented Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Changes

Verification

Uh oh!

Troy-Ferrell commented Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keveleigh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Troy-Ferrell commented Nov 11, 2019

Uh oh!

Troy-Ferrell commented Nov 11, 2019

Uh oh!

azure-pipelines bot commented Nov 11, 2019

Uh oh!

keveleigh commented Dec 12, 2019

Uh oh!

Troy-Ferrell commented Dec 12, 2019

Uh oh!

Troy-Ferrell commented Dec 12, 2019

Uh oh!

azure-pipelines bot commented Dec 12, 2019

Uh oh!

keveleigh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Troy-Ferrell commented Dec 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Troy-Ferrell commented Dec 16, 2019

Uh oh!

azure-pipelines bot commented Dec 16, 2019

Uh oh!

Troy-Ferrell commented Dec 16, 2019

Uh oh!

keveleigh commented Dec 17, 2019

Uh oh!

azure-pipelines bot commented Dec 17, 2019

Uh oh!

keveleigh commented Dec 17, 2019

Uh oh!

azure-pipelines bot commented Dec 17, 2019

Uh oh!

Troy-Ferrell commented Dec 17, 2019

Uh oh!

azure-pipelines bot commented Dec 17, 2019

Uh oh!

Troy-Ferrell commented Dec 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Troy-Ferrell commented Nov 6, 2019 •

edited

Loading

Troy-Ferrell commented Nov 6, 2019 •

edited

Loading

Troy-Ferrell commented Dec 16, 2019 •

edited

Loading