search

r-lib / gh

Minimalistic GitHub API client in R
https://gh.r-lib.org
Other
223 stars 52 forks source link

URL-based, credentials-backed token lookup #123

Closed jennybc closed 4 years ago

jennybc commented 4 years ago

WIP

Closes #120 Use credentials::git_credential_ask() to summon the token for a given host Closes #110 Token: check for env var first, then the keyring

The best place to start your review is probably by reading the new vignette, which (re-)orients you to how things work in gh and explains aspects of credentials that I have wired in. Also raises a few questions and TODOs.

@jeroen This is going to look very familiar, because set_github_pat2() here is similar to https://github.com/r-lib/credentials/pull/7. I'm not actually proposing this function name. I still think it's possible that this function belongs in credentials? In any case, that needs to be discussed in order to pick a name and decide where/if to export.

I have confirmed that things "just work" for me with GitHub.com and GitHub.ubc.ca.

jennybc commented 4 years ago

A really big question to me is:

Where should this function live? credentials or gh?

If credentials, does it replace / generalize credentials::set_github_pat()? @jeroen this will make you chuckle since you successfully fended off https://github.com/r-lib/credentials/pull/7 but the new element now is that gh already had this env var based system for keeping a Github.com PAT separate from a PAT for Github.acme.com. Maybe this should be credentials::set_github_pat()?

If gh, what do we call it? set_github_pat2() was a deliberately awful choice that cannot stick.

gaborcsardi commented 4 years ago

Where should this function live? credentials or gh?

They are fine here, I think. Jeroen can always add them to credentials later if he wants.

jennybc commented 4 years ago

I'm going to close this, now that the URL aspects seem likely to appear in credentials.

I'll be back with yet another PR after that to implement the bits that still remain in gh.