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

Support passthrough of credentials to git2r #982

Closed
wants to merge 58 commits into from

Conversation

onlymee
Copy link

@onlymee onlymee commented Nov 18, 2015

git2r has very sensible defaults for credentials, eg. for ssh it defaults to looking for ssh-agent, but there are times where it is useful to specify specific credentials.

This modification simply allows for a git2r credentials object to passet through to the underlying clone call. This is useful if you wish to use devtools non-interactively (e.g. a script in a CI solution)

@onlymee
Copy link
Author

onlymee commented Nov 18, 2015

AppVeyor test is failing because it's hitting the Github API Rate limit while running the tests.

@@ -18,20 +20,21 @@
#' install_git("git://github.com/hadley/stringr.git")
#' install_git("git://github.com/hadley/stringr.git", branch = "stringr-0.2")
#'}
install_git <- function(url, subdir = NULL, branch = NULL, args = character(0),
install_git <- function(url, subdir = NULL, branch = NULL, credentials=NULL, args = character(0),
Copy link
Member

Choose a reason for hiding this comment

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

Please put spaces around = to match existing style

Copy link
Member

Choose a reason for hiding this comment

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

Bump

Copy link
Member

Choose a reason for hiding this comment

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

@hadley would you like same thing done for use_github()?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, if it would be helpful

… install_remote has FALSE for that argument, and short circuit so install is only called once
…o other temp directories and fix the missing bracket in test
…e DESC file

updated the test expectation to match what actually gets returned
…ll as better handling lack of sha1 and or user / remote
…ctions separate so that can make independent changes if needed later
…they should just give the same metadata, and no reason to be separate functions
…he setting to NULL in else statements to make it a little clearer
@hadley
Copy link
Member

hadley commented Jan 20, 2016

If you can do this by Friday it'll make it in 1.10, otherwise it will have to wait for 1.11 (probably in about a months time)

hadley and others added 22 commits January 20, 2016 16:49
…version of the package, and add a test using load_all() followed by install as this was causing a failure previously
@onlymee
Copy link
Author

onlymee commented Jan 22, 2016

Also added the credentials passthrough to use_github as suggested. I checked for other git2r clone/push/fetch but missed that one.

This could probably have been 1 or 2 commits - will chalk that down to experience.

@jimhester
Copy link
Member

@onlymee you can squash your commits with git rebase -i to combine them. See https://help.github.com/articles/about-git-rebase/.

You should also rebase this PR against the current master first which would remove a lot of the above commits. If you have 'hadley/devtools' as the remote called upstream then git rebase upstream master when you are on your master branch should do it.

@hadley
Copy link
Member

hadley commented Mar 23, 2016

Can you please rebase & squash?

@jimhester
Copy link
Member

I rebased, squashed and merged at 91fddbf. Thanks @onlymee!

@jimhester jimhester closed this Apr 11, 2016
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.

5 participants