Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use authentication in GitPreparer.GetRemoteReference #468

Merged
merged 1 commit into from Jun 17, 2015
Merged

Use authentication in GitPreparer.GetRemoteReference #468

merged 1 commit into from Jun 17, 2015

Conversation

johannesegger
Copy link

You already had a helper for this (GitHelper.GetRemoteTipsUsingUsernamePasswordCredentials), but it was internal. I made it public.

I saw somewhere else that you used repository.Network.Remotes.Single(), which fails when there are multiple remotes, so I'm not sure if it's ok that I also used it.

@JakeGinnivan
Copy link
Contributor

@nulltoken can you have a quick look over this one?

@@ -164,7 +164,7 @@ static void CreateFakeBranchPointingAtThePullRequestTip(Repository repo, Authent
repo.Checkout(fakeBranchName);
}

static IEnumerable<DirectReference> GetRemoteTipsUsingUsernamePasswordCredentials(Repository repo, Remote remote, string username, string password)
public static IEnumerable<DirectReference> GetRemoteTipsUsingUsernamePasswordCredentials(Repository repo, Remote remote, string username, string password)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why public rather than internal?

Copy link
Author

Choose a reason for hiding this comment

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

Right, I overlooked that GitVersionCore internals are visible in GitVersionExe.

@nulltoken
Copy link
Contributor

@nulltoken can you have a quick look over this one?

Sir! Done, Sir!

@johannesegger
Copy link
Author

Thanks for your feedback.

I now added an overload of GitHelper.GetRemoteTipsUsingUsernamePasswordCredentials that creates a temporary remote and calls the original GetRemoteTipsUsingUsernamePasswordCredentials with this remote. I used a Guid for the remote name. Although it's not perfect, it should be ok most of the time.

@nulltoken
Copy link
Contributor

🆒 Looks 👍 to me.

JakeGinnivan added a commit that referenced this pull request Jun 17, 2015
Use authentication in `GitPreparer.GetRemoteReference`
@JakeGinnivan JakeGinnivan merged commit 2201f0d into GitTools:master Jun 17, 2015
@JakeGinnivan
Copy link
Contributor

Thanks for your help @nulltoken!

@nulltoken
Copy link
Contributor

@JakeGinnivan As always, feel free to ping me!

@johannesegger johannesegger deleted the use-authentication branch June 17, 2015 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants