10000 Fix opening links on enterprise instances. by grokys · Pull Request #1309 · 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.

Fix opening links on enterprise instances. #1309

Merged
merged 1 commit into from
Nov 10, 2017

Conversation

grokys
Copy link
Contributor
@grokys grokys commented Nov 9, 2017

Correctly compare repository full name.

Previously we were comparing repo.FullName == ActiveRepo.Name which always failed as repo.FullName returns owner/name whereas ActiveRepo.Name just returns the name portion. Construct a full name for the active repo and check that.

Fixes #1291

Fixes opening links on enterprise instances.

Previously we were comparing `repo.FullName == ActiveRepo.Name` which always failed as `repo.FullName` returns `owner/name` whereas `ActiveRepo.Name` just returns the `name` portion. Construct a full name for the active repo and check that.

Fixes #1291
@@ -131,7 +131,8 @@ void RefreshRepo()
if (!isdotcom)
{
var repo = await SimpleApiClient.GetRepository();
return (repo.FullName == ActiveRepo.Name || repo.Id == 0) && SimpleApiClient.IsEnterprise();
var activeRepoFullName = ActiveRepo.Owner + '/' + ActiveRepo.Name;
return (repo.FullName == activeRepoFullName || repo.Id == 0) && SimpleApiClient.IsEnterprise();
Copy link
Contributor Author
@grokys grokys Nov 9, 2017

Choose a reason for hiding this comment

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

I'm not sure what the check for repo.Id == 0 is needed for here?

@meaghanlewis
Copy link
Contributor

This LGTM 👍 . Thanks for the fix @grokys

Copy link
Collaborator
@jcansdale jcansdale left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@jcansdale jcansdale merged commit 554b193 into master Nov 10, 2017
@jcansdale jcansdale deleted the fixes/1291-open-on-github-enterprise branch November 10, 2017 18:24
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
0