Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Making TeamExplorerServiceHolder testable #1292

Merged
merged 4 commits into from
Nov 21, 2017

Conversation

shana
Copy link
Contributor

@shana shana commented Nov 6, 2017

This makes TeamExplorerServiceHolder testable by moving/abstracting all the dependent VS types.

This also reverts the change to have GitHub.TeamFoundation.15 reference Shell.15 - we rely on types from Shell.14 in GitHub.Exports and other areas (Shell.14 is available in VS 2017), namely UIContextChangedEventArgs.

@shana shana requested review from jcansdale and grokys November 6, 2017 03:54
@grokys
Copy link
Contributor

grokys commented Nov 8, 2017

When running this I get:

Exception thrown: 'GitHub.GitHubLogicException' in GitHub.Exports.dll

Additional information: Could not locate view model for GitHubPane.

@jcansdale
Copy link
Collaborator

This also reverts the change to have GitHub.TeamFoundation.15 reference Shell.15

The problem is that Microsoft.TeamFoundation.Git.Provider, Version 15.0.0.0 (which we reference from GitHub.TeamFoundation.15), references the Microsoft.VisualStudio.Shell.15.0 assembly.

This causes the following warning.

C:\Program Files (x86)\MSBuild\14.0\bin\Microsoft.Common.CurrentVersion.targets(1820,5):
warning MSB3277: Found conflicts between different versions of the same dependent assembly that
could not be resolved.  These reference conflicts are listed in the build log when log verbosity is set to detailed.
[C:\source\github.com\github\VisualStudio\src\GitHub.TeamFoundation.15\GitHub.TeamFoundation.15.csproj]

These warnings can be difficult to track down and it isn't at all clear what the issue is whether it's serious or not.

@jcansdale
Copy link
Collaborator

I keep getting the following on this branch:

image

I wonder what's going on?

@shana
Copy link
Contributor Author

shana commented Nov 17, 2017

This should be working now, MEF crashes are fixed.

Also, this branch is now loading the following DLLs up front when starting VS in Team Explorer Home:

image

Note that GitHub.Exports.Reactive, GitHub.UI.Reactive and GitHub.Extensions.Reactive are not on the list, which is good! But rx is still getting loaded, probably via GitHub.App, so we should continue spelunking to find out what's loading GitHub.App so we stop doing that (this might be the credential handler, in which case #1277 might fix that.

@shana shana force-pushed the fixes/testable-teserviceholder branch from 171e52a to 25119fc Compare November 17, 2017 14:15
@shana shana force-pushed the fixes/testable-teserviceholder branch from 25119fc to 84df182 Compare November 17, 2017 14:16
grokys
grokys previously approved these changes Nov 21, 2017
Copy link
Contributor

@grokys grokys left a comment

Choose a reason for hiding this comment

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

Approved, with one nit.

ActiveRepositoriesChanged?.Invoke();
}

public IEnumerable<ILocalRepositoryModel> ActiveRepositories => gitService?.ActiveRepositories.Select(x => IGitRepositoryInfoExtensions.ToModel(x));
Copy link
Contributor

Choose a reason for hiding this comment

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

IGitRepositoryInfoExtensions.ToModel is an extension method so why not just say x.ToModel()?

Copy link
Contributor

@grokys grokys left a comment

Choose a reason for hiding this comment

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

Fixed the nit so 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants