Skip to content

Create a way to configure git protocol and git2r credentials#653

Merged
jennybc merged 9 commits intomasterfrom
credentials-and-protocol
Mar 14, 2019
Merged

Create a way to configure git protocol and git2r credentials#653
jennybc merged 9 commits intomasterfrom
credentials-and-protocol

Conversation

@jennybc
Copy link
Member

@jennybc jennybc commented Mar 13, 2019

Gives us:

  • git_protocol() / use_git_protocol() to summon or set a default protocol (so, either SSH or HTTPS)
  • git2r_credentials() / use_git2r_credentials() to summon or set git2r credentials

This addresses your earlier distaste @hadley for getting and setting protocol with the same function, i.e. I've split it out and followed same design for credentials.

Clears the way for me to tackle #533 Build protocol flexibility into pr_*() functions

@jennybc jennybc requested a review from hadley March 13, 2019 20:49
@jennybc jennybc merged commit 3b8f228 into master Mar 14, 2019
@jennybc
Copy link
Member Author

jennybc commented Mar 14, 2019

@cderv @ijlyttle With this merged, I think we are going to have much more success with HTTPS and on Windows, because I make a git2r credential from the GitHub PAT whenever possible. Now I will use this to wire this all in to the PR flow.

But starting now, it would be great for you two to test drive this as much as possible. It's nearly impossible to test. I do some manual tests but I don't cover many scenarios.

@jennybc jennybc deleted the credentials-and-protocol branch March 14, 2019 19:07
@cderv
Copy link
Contributor

cderv commented Mar 17, 2019

First try with a ˋpr_fetch` and first error. Something does not work as intended with git2r::fetch and credentials.
I need to dig a little further to isolate the issue.

@ijlyttle
Copy link
Contributor

So far, I have succeeded, no problems, on macOS with: pr_init(), pr_push(), and pr_fetch().

I'll use more of the functions today and try things with Windows.

@cderv
Copy link
Contributor

cderv commented Mar 18, 2019

ok my bad, this is working now with last pr updates. Will continue to test and open specific issues if needed.

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.

4 participants