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

Fixed #627: Project recently published to remote pre-fills local name for new project #664

Merged
merged 9 commits into from
Dec 16, 2016

Conversation

jswietek
Copy link
Contributor

@jswietek jswietek commented Nov 4, 2016

2 main changes:

  1. RepositoryPublishService now has a reference to VSGitService. This is used to call GetActiveRepo() everytime when needed instead of getting that value in constructor. This is needed as active repo may change and the reference was never updated in the RepositoryPublishService

  2. GitHubPublishSection.cs now re-initialize the section view when active repo changes.

Fixes #627

Copy link
Contributor

@tocsoft tocsoft left a comment

Choose a reason for hiding this comment

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

found some white space issues

var view = new GitHubInvitationContent();
SectionContent = view;
view.DataContext = this;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

these should be spaces not tabs

@@ -77,7 +82,8 @@ protected override void RepoChanged(bool changed)
{
base.RepoChanged(changed);
Setup();
}
InitializeSectionView();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

as above these should be spaces not tabs.

Changed indentation from tabs to spaces
if (!string.IsNullOrEmpty(activeRepository?.Info?.WorkingDirectory))
return new DirectoryInfo(activeRepository.Info.WorkingDirectory).Name ?? "";
if (!string.IsNullOrEmpty(vsGitServices.GetActiveRepo()?.Info?.WorkingDirectory))
return new DirectoryInfo(vsGitServices.GetActiveRepo().Info.WorkingDirectory).Name ?? "";
Copy link
Contributor

Choose a reason for hiding this comment

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

GetActiveRepo is a somewhat heavy operation, so it's probably best to cache the value here to avoid the double hit on it.

@shana
Copy link
Contributor

shana commented Nov 8, 2016

This is awesome stuff, thank you! I just had a little comment about the performance of the property getter, otherwise it's great! ✨

@grokys
Copy link
Contributor

grokys commented Dec 2, 2016

Hi @jswietek - due to some recent changes we had to make for VS2017 this is now failing for me because this line:

https://github.com/github/VisualStudio/pull/664/files#diff-a1cf7690606825b92e0c421049f01872R43

Is being called on a background thread, and it ends up invoking AsyncPackage.GetService which must be called from the main thread. Did you want to try fixing this, or do you want us to take it from here?

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.

4 participants