-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Remove RepositoryHost(s) #1283
Remove RepositoryHost(s) #1283
Conversation
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 ` 8000 TrackingCollection` implement `IReadOnlyObservableCollection`.
Following advice by @shana.
This services the same purpose as `IConnectionRepositoryHostMap` served when `RepositoryHost` was still around: to provide access to the globally registered connection.
After removing the contents of the: |
…local-repositories
…connection-manager Conflicts: src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs
|
||
[ImportingConstructor] | ||
public ModelServiceFactory( | ||
IApiClientFactory apiClientFactory, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to use ISimpleApiClientFactory
here, so we can slowly move away from IApiClientFactory
and get rid of it.
There was a problem hiding this comment.
Choose a reason for 8000 hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but ModelService
still relies on IApiClient
rather than ISimpleApiClient
and needs the reactive stuff, so if we want to make that change it will have to be later I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just had a small comment about the usage of IApiClientFactory
(which we should really move away from)
When creating a `ModelService`, ensure that the correct `AccountCacheItem` is inserted.
…local-repositories Conflicts: src/GitHub.VisualStudio/Services/ConnectionManager.cs
…connection-manager Conflicts: src/GitHub.App/Models/RepositoryHost.cs src/GitHub.App/Models/RepositoryHosts.cs src/GitHub.App/ViewModels/PullRequestListView 8000 Model.cs src/GitHub.VisualStudio/Services/ConnectionManager.cs
…remove-repositoryhosts Conflicts: src/GitHub.App/Models/RepositoryHost.cs src/GitHub.App/Models/RepositoryHosts.cs src/GitHub.App/Services/ModelService.cs src/GitHub.App/ViewModels/GistCreationViewModel.cs src/GitHub.App/ViewModels/LoginTabViewModel.cs src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs src/GitHub.App/ViewModels/PullRequestListViewModel.cs src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs
this PR looks good to me 👍 |
This PR removes
RepositoryHosts
andRepositoryHost
and makes all clients that used them useConnectionManager
instead.Current state: I think mostly done but not tested much.
Depends on #1280