8000 Removed `IConnection.Repositories`. by grokys · Pull Request #1279 · github/VisualStudio · GitHub
[go: up one dir, main page]

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

Removed IConnection.Repositories. #1279

Merged

Conversation

grokys
Copy link
Contributor
@grokys grokys commented Oct 20, 2017

Removed IConnection.Repositories and IConnectionManager.RefreshRepositories and replaced them with LocalRepositories which acts as a central place to find known repositories.

As part of this, also added IReadOnlyObservableCollection and ObservableCollectionEx because the BCL's ReadOnlyObservableCollection sucks. Made TrackingCollection implement IReadOnlyObservableCollection.

Removed `IConnection.Repositories` and `IConnectionManager.RefreshRepositories` and replaced them with `LocalRepositories` which acts as a central place to find known repositories.

As part of this, also added `IReadOnlyObservableCollection` and `ObservableCollectionEx` because the BCL's `ReadOnlyObservableCollection` sucks. Made `TrackingCollection` implement `IReadOnlyObservableCollection`.
/// Explorer.
/// </remarks>
[Export(typeof(ILocalRepositories))]
public class LocalRepositories : ILocalRepositories
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't this this implementation should be here, this will bring all the Rx dependencies in when there's no need for it. The reason this was in ConnectionManager to begin with was meant to be light weight and require minimal dependencies. Perhaps GitHub.Exports or some other place would be better? Maybe even GitHub.VisualStudio?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't the fact that its export is in GitHub.Exports good enough? If you look at some of our models, e.g. PullRequestModel - that is defined in GitHub.App with its interface in GitHub.Exports meaning that it's available to the non-rx parts of the extension. Rx shouldn't be loaded until a type that references rx is loaded, right?

grokys and others added 3 commits October 24, 2017 12:20
@grokys grokys merged commit 8f6d218 into refactor/connections-master Nov 8, 2017
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.

2 participants
0