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

Update internal interface usage for FileExplorer integration #3151

Conversation

DefaultRyan
Copy link
Member

Summary of the pull request

This updates the Microsoft.Internal.Windows.DevHome.Helpers package version to the latest with the new interfaces for File Explorer integration. The associated interface implementation/wrappers are also updated here.

References and relevant issues

As the OS changes with the new interfaces propagate and we near public preview, we should be able to deprecate and remove the old testing/prototyping interface.

Validation steps performed

Built and installed this version of DevHome on Windows VMs:

  • Only knew about the old testing interfaces.
  • Updated builds that check for the new official interfaces.

@DefaultRyan DefaultRyan requested a review from ssparach June 6, 2024 18:23
@@ -57,7 +57,7 @@ private static void HandleCOMServerActivation()
Log.Information($"Activating COM Server");
using var sourceControlProviderServer = new SourceControlProviderServer();
var sourceControlProviderInstance = new SourceControlProvider();
var wrapper = new Microsoft.Internal.Windows.DevHome.Helpers.FileExplorer.PerFolderRootSelectorWrapper(sourceControlProviderInstance);
var wrapper = new Microsoft.Internal.Windows.DevHome.Helpers.FileExplorer.ExtraFolderPropertiesWrapper(sourceControlProviderInstance, sourceControlProviderInstance);
Copy link
Contributor

Choose a reason for hiding this comment

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

Quick Question: Why is the same object instance sent twice as function argument? Is this related to the update in interface where one interface provides property information on folder and the other provides property information on files stored under the folder? Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is related to the interface update. One is for the legacy testing interface, and the other is for the new interface we'll use going forward. Rather than query at runtime for the existence of one of the interfaces (which could fail), I simply decided to provide two params to force the compiler to check for us. Once the helpers package can wean itself off the legacy testing interface, I'll delete the second param from that constructor and make the corresponding update here.

@@ -17,7 +17,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="LibGit2Sharp" Version="0.29.0" />
<PackageReference Include="LibGit2Sharp" Version="0.30.0" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this version changing?

Copy link
Member Author

Choose a reason for hiding this comment

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

There are some improvements in this latest version, but probably the most important one is that it updates LibGit2 to 1.7.2, which includes a couple security fixes and is recommended by the LibGit2 maintainers. https://github.com/libgit2/libgit2/releases/tag/v1.7.2

@DefaultRyan DefaultRyan merged commit edef352 into feature/fileexplorer-sourcecontrol-integration Jun 27, 2024
4 checks passed
@DefaultRyan DefaultRyan deleted the user/ryansh/fileexplorer-interface-update branch June 27, 2024 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants