Make InlineReviewsPackage async. #1497
Conversation
| await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync(); | ||
| var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); | ||
| var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); | ||
| PackageResources.Register(this, componentModel.DefaultExportProvider, menuService); |
There was a problem hiding this comment.
It would be better to go through GitHubServiceProvider for this. GitHubServiceProvider already loads up SComponentModel when it gets initialized and uses it for lookups, guaranteeing that only types that are ours get returned in MEF queries.
Going through GitHubServiceProvider also means that we can bypass the ExportProvider look up at any time by precaching instances in it ahead of time (which is handy for tests and also if we want to avoid a MEF query for some reason).
There was a problem hiding this comment.
The problem with GitHubServiceProvider is that it doesn't support async lookups, and given that we're in InitializeAsync I figured that we really should be doing these async. Or have I missed something?
|
@shana made the change we discussed. |
jcansdale
left a comment
jcansdale
left a comment
There was a problem hiding this comment.
I think we should be using PackageAutoLoadFlags.BackgroundLoad.
| [PackageRegistration(UseManagedResourcesOnly = true)] | ||
| [PackageRegistration(UseManagedResourcesOnly = true, AllowsBackgroundLoading = true)] | ||
| [Guid(Guids.InlineReviewsPackageId)] | ||
| [ProvideAutoLoad(UIContextGuids80.SolutionExists)] |
There was a problem hiding this comment.
This will cause the package to auto-load on the Main thread. I think we should be using this:
[ProvideAutoLoad(UIContextGuids80.SolutionExists, PackageAutoLoadFlags.BackgroundLoad)]
In future we might want to only auto-load when there is an active Git repository (see #1512)?
[ProvideAutoLoad(Guids.UIContext_Git, PackageAutoLoadFlags.BackgroundLoad)]
|
Superseded by #1502. |
I used the VS2017 project system package as a template for the required attributes/procedure for this.
This required a bit of jiggling of the command infrastructure we have in the
GitHub.InlineReviewspackage. This infrastructure was intended to be temporary, pending a command infrastructure that merges that inGitHub.VisualStudio. I'm looking into doing that right now, so don't dwell too much on that part of it.Fixes #1489.