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.

Sign up for GitHub

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

Jump to bottom

Environment Manager is now RS5+. Tracking Registry writes is Windows 11+ and with better checks. #2938

Merged
merged 11 commits into from
Oct 10, 2022
Merged

Environment Manager is now RS5+. Tracking Registry writes is Windows 11+ and with better checks. #2938

merged 11 commits into from
Oct 10, 2022

Conversation

dhoehna
Copy link
Contributor

@dhoehna dhoehna commented Sep 6, 2022 •
edited
Loading

The Spec PR is here: #2544

This PR addresses two problems with the EM API.

  1. The API will work on RS5+
  2. EM did not check for the existence of the Windows app sdk framework package to decide if tracking is enabled.

This PR fixes these issues by

  1. IsSupported returns true.
  2. A new non-static method AreChangesTrackedis added to query if the registry changes will be stored.

In addition to the above changes the this PR also did

  1. Removed the UWP tests.
  2. Add more comments, and fixes to the BuildTransportPackage powershell script.

@ghost ghost added the needs-triage label Sep 6, 2022
@dhoehna
Copy link
Contributor Author

dhoehna commented Sep 6, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dhoehna dhoehna requested a review from kythant September 6, 2022 19:03
@dhoehna
Copy link
Contributor Author

dhoehna commented Sep 12, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dhoehna
Copy link
Contributor Author

dhoehna commented Oct 3, 2022

Self contained checks work.

Changes will not be tracked if Windows App SKD is self-contained because windowsAppSDK can not guarantee that the framework bits will be loaded at uninstall time to revert any changes.

@dhoehna
Copy link
Contributor Author

dhoehna commented Oct 4, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

DrusTheAxe
DrusTheAxe approved these changes Oct 6, 2022
View reviewed changes
Copy link
Member

@DrusTheAxe DrusTheAxe left a comment

Choose a reason for hiding this comment

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

LGTM modulo some comments

@dhoehna dhoehna enabled auto-merge (squash) October 10, 2022 20:20
@dhoehna
Copy link
Contributor Author

dhoehna commented Oct 10, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dhoehna dhoehna merged commit c359bc1 into main Oct 10, 2022
@dhoehna dhoehna deleted the user/dahoehna/MakingEMAPIWorkOnRS5+ branch October 10, 2022 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonwis jonwis jonwis left review comments

@DrusTheAxe DrusTheAxe DrusTheAxe approved these changes

@EricJohnson327 EricJohnson327 Awaiting requested review from EricJohnson327

@kythant kythant Awaiting requested review from kythant

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dhoehna @DrusTheAxe @jonwis @bpulliam