Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up[Feature] Show current PR on status bar #1396
Conversation
This isn't pretty but gets the UI control to where we want it.
Initialize with InlineReviewsPackage. This should probably initialize earlier from its own package.
Remove view when there is no PR session.
Show current PR on status bar and allow navigation to PR details MVP
| this.usageTracker = usageTracker; | ||
| this.serviceProvider = serviceProvider; | ||
|
|
||
| OnActiveRepositoriesChanged(); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Just a heads up, I want to focus on some stuff around the code review workflow and won't be able to fix the icon color soon. Since it's not a critical blocker for this feature, we could consider merging if this PR looks good otherwise, and I can address it in a separate issue / PR. |
| public VSGitExt(IGitHubServiceProvider serviceProvider) | ||
| : this(serviceProvider, new VSUIContextFactory(), new LocalRepositoryModelFactory()) | ||
| public VSGitExt(Func<Type, Task<object>> getServiceAsync) | ||
| : this(getServiceAsync, new VSUIContextFactory(), new LocalRepositoryModelFactory()) |
This comment has been minimized.
This comment has been minimized.
shana
Mar 5, 2018
•
Collaborator
Any reason to send a callback instead of sending in IAsyncServiceProvider (which provides GetServiceAsync) here?
This comment has been minimized.
This comment has been minimized.
jcansdale
Mar 5, 2018
•
Author
Collaborator
The issue was that AsyncPackage implements 3 interfaces called IAsyncServiceProvider, 2 of which exist in the same namespace. Passing a delegate was a way to avoid taking a new dependency on Microsoft.VisualStudio.Shell.Immutable.14.0 and needing to disambiguate using extern alias (which works but is a bit messy).
I've pushed a commit that uses the interface from Microsoft.VisualStudio.Shell.Immutable.14.0. If we're likely to be using IAsyncServiceProvider elsewhere, having the correct assembly to hand is maybe a good thing (even if we need to use extern alias).
An alternative approach would be to use the lower level Interop.IAsyncServiceProvider interface from here:
namespace Microsoft.VisualStudio.Shell.Interop
{
public interface IAsyncServiceProvider
{
IVsTask QueryServiceAsync([ComAliasName("OLE.REFGUID")] ref Guid guidService);
}
}
The fact that this takes a Guid and returns a IVsTask again makes it a bit messy.
My instinct is to go with this last commit. What do you think?
Use IAsyncServiceProvider from the Microsoft.VisualStudio.Shell.Immutable.14.0 assembly.
Which two assemblies provide the same type? |
|
It's a collision between
I'll see what happens if I remove the |
Use an alias on the up-level assembly rather than the shared one. We're likely to want to use IAsyncServiceProvider from Microsoft.VisualStudio.Shell.Immutable.14 again, so give alias to Microsoft.VisualStudio.Shell.Framework instead.
|
I've changed it so that rather than giving an alias to the shared This will make finding the correct |

jcansdale commentedDec 18, 2017
•
edited
The following PRs target this one:
Functionality
To do
[ProvideAutoLoad(Guids.GitSccProviderId)]?Use PR number from branch metadata?IPullRequestSessionManager, see https://github.com/github/VisualStudio/pull/1388/files#diff-8f8b4cd719001e4a8dd4b96e211243a8R53Related issues