Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add url parameter in set_github_pat()#9

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
jeroen wants to merge3 commits intomain
base:main
Choose a base branch
Loading
frompathost
Draft

Conversation

@jeroen
Copy link
Member

@jeroenjeroen commentedAug 6, 2020
edited
Loading

Many open questions:

  • Should we support usernames in the url? Not all managers allow this, GCM-core always overrides the username. Update: gabor suggests usingGCM_AUTHORITY=Basic.
  • Should we slugify the username? I think not, so that I can doset_github_pat(url = "https://dummy@github.com") and have that as my newGITHUB_PAT.
  • Currently we require the full API in the url, so the user needs to specify like:set_github_pat(url = "https://github.ucla.edu/api/v3/"). Alternatively we could let the user just specify thehostname part from thegithub enterprise docs because everything else is fixed.

@jeroenjeroen marked this pull request as draftAugust 6, 2020 23:00
Copy link
Member

@jennybcjennybc left a comment
edited
Loading

Choose a reason for hiding this comment

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

Currently we require the full API in the url, so the user needs to specify like:set_github_pat(url = "https://github.ucla.edu/api/v3/")`. Alternatively we could let the user just specify the hostname part from the github enterprise docs because everything else is fixed.

I would really like to allow this:

So I'd likeurl to be accepted in "browser" or "API" form. But I also don't want to have to pre-strip the protocol. Similar changes are in the dev version of gh now.

return(do.call(Sys.setenv, structure(list(cred$password),names=pat_var)))
}
}
if(verbose==TRUE){
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick

Suggested change
if(verbose==TRUE){
if(isTRUE(verbose)){

pat_url<- sprintf('https://%s@github.com',pat_user)
set_github_pat<-function(force_new=FALSE,url='https://api.github.com',
validate= interactive(),verbose=validate){
url_data<- get_pat_credential_url(url)
Copy link
Member

Choose a reason for hiding this comment

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

Would a name involving "parse" or similar be more self-explanatory?

# safely parse _AT_ to look for custom usernames.
get_pat_credential_url<-function(url){
# strip protocol
url<- sub("^[a-z]+://","", tolower(url))
Copy link
Member

Choose a reason for hiding this comment

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

Thistolower() is in tension with what gh has been doing when forming these env var names.

pat_var<-if(grepl("github.com$",url)){
'GITHUB_PAT'
}else {
paste0('GITHUB_PAT_', gsub("\\.","_", sub("^.+@","",url)))
Copy link
Member

Choose a reason for hiding this comment

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

gh doestoupper(url) (modulo the fact that it does not have any notion of such a URL containing a username).

@jennybc
Copy link
Member

As for supporting and slugifying username ... we could NOT support that initially, in order to make it easier to move forward now.

Getting support for non-github.com deployments feels like 90% of the win here and we can wait and see if people actually ask for username support.

@jennybc
Copy link
Member

jennybc commentedAug 14, 2020
edited
Loading

For the Windows Git Credential Manager, the GCM_INTERACTIVE environment variable is very intriguing to me:

http://microsoft.github.io/Git-Credential-Manager-for-Windows/Docs/Environment.html#gcm_interactive

Specifies if user can be prompted for credentials or not.

Supports Auto, Always, or Never. Defaults to Auto.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jennybcjennybcjennybc left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jeroen@jennybc

[8]ページ先頭

©2009-2025 Movatter.jp