8000 Remove RepositoryHost(s) by grokys · Pull Request #1283 · 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.

Remove RepositoryHost(s) #1283

Merged

Conversation

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

This PR removes RepositoryHosts and RepositoryHost and makes all clients that used them use ConnectionManager instead.

Current state: I think mostly done but not tested much.

Depends on #1280

grokys added 13 commits October 20, 2017 17:52
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`.
This services the same purpose as `IConnectionRepositoryHostMap` served when `RepositoryHost` was still around: to provide access to the globally registered connection.
@grokys grokys changed the title WIP: Remove RepositoryHost(s) Remove RepositoryHost(s) Nov 3, 2017
@jcansdale
Copy link
Collaborator

After removing the contents of the: C:\Users\<user>\AppData\Local\GìtHūbVisualStudio directory, the login stuck on GitHub Enterprise and I wasn't able to log into my GitHub account. I installed this PR version and tried again. After this I was able to log in successfully. 👍

grokys and others added 2 commits November 3, 2017 17:12
…connection-manager

 Conflicts:
	src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs
	src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs

[ImportingConstructor]
public ModelServiceFactory(
IApiClientFactory apiClientFactory,
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor
@shana shana left a 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
@meaghanlewis
Copy link
Contributor

this PR looks good to me 👍

@grokys grokys changed the base branch from refactor/connection-manager to refactor/connections-master November 8, 2017 07:20
@grokys grokys merged commit def6070 into refactor/connections-master Nov 8, 2017
@grokys grokys deleted the refactor/connections/remove-repositoryhosts branch November 8, 2017 07:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
4B65
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0