Conversation
mislav
left a comment
There was a problem hiding this comment.
Works great on this end! 🎉
I only have nitpicks about edge-cases and docs
| }, | ||
| } | ||
|
|
||
| cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The GitHub host to use for authentication") |
There was a problem hiding this comment.
Would it be appropriate to list github.com here as a default value for this flag?
There was a problem hiding this comment.
I wanted people to interactively select a host if none was passed and this seemed like the correct approach. if this default is specified, in refreshRun it would not be possible to tell if a user passed nothing (and thus I should prompt when multiple hosts are configured).
In an old style command I'd just check .Changed(), but we make a point of not accessing the flags directly in the run portion of commands.
There was a problem hiding this comment.
Makes sense! BTW, in cases when we still need to check Changed(), we do and save that information to an Options struct
cli/pkg/cmd/issue/create/create.go
Lines 62 to 63 in 26a5f11
But you have a good point about the interactive selection for a host happening, and in that case github.com is not the "default" (except that it likely appears first in the list).
There was a problem hiding this comment.
That was a valid path too (adding a *Provided) but I was seeing how not cluttering up the opts struct felt.
|
|
||
| candidates, err := cfg.Hosts() | ||
| if err != nil { | ||
| return fmt.Errorf("not logged in to any hosts. Use 'gh auth login' to authenticate with a host") |
There was a problem hiding this comment.
If I have explicitly chosen a host, would it make sense that the operation continues instead of erroring out when there is no pre-existing hosts configuration?
$ rm ~/.config/gh/hosts.yml
$ gh auth refresh -h github.com
not logged in to any hosts. Use 'gh auth login' to authenticate with a host
There was a problem hiding this comment.
It seemed wrong to have refresh replace login, here. If a user does the initial authentication here they won't get the git_protocol prompt nor any additional "first auth" setup we add later. I preferred refresh and login having distinct purposes.
@ampinsk what do you think?
There was a problem hiding this comment.
yeah, I prefer keeping them distinct too! 👍
This PR adds
gh auth refresh, a command for adding new scopes to an existing token.It errors if
GITHUB_TOKENis set in the environment, if no hosts are authenticated, if--hostnameis specified for a hostghdoesn't know about, or ifghis not connected to a TTY.Part of #1413