feat: allow git remote names in gh repo set-default #12377
Conversation
When specifying a repository for `gh repo set-default`, users can now use a git remote name (e.g., "origin", "upstream") instead of the full OWNER/REPO format. The command first checks if the argument is a git remote name. If found, it uses the corresponding repository. Otherwise, it falls back to parsing the argument as OWNER/REPO format. Example: gh repo set-default origin Fixes cli#9149 Signed-off-by: majiayu000 <1835304752@qq.com>
babakks
left a comment
There was a problem hiding this comment.
Thanks for the PR, @majiayu000! 🙏 It's almost there, except for a small modification.
I haven't yet fully reviewed the tests, though.
babakks
left a comment
There was a problem hiding this comment.
Thanks for the fix, @majiayu000! 🍻
We just need to get rid of the redundant if-statement (see the first/only comment), and then I think we're good to ship this.
Just as a bit of context, most gh commands work in a two-stage kind of way: 1) pre-run (e.g. arg/flag validation, which are done within cmd.RunE), and 2) run (done in a top-level runX function). In most cases, we restrict the pre-run stage to only static/offline checks that do not need any API calls, or git invocations.
The repo set-default command has already been a bit of an exception due to the call to opts.GitClient.IsLocalGitRepo which invokes git rev-parse under the hood. Now with the new remote lookup step (in cmd.RunE) we're bending the rules a bit more.
That said, I'm not going to ask you for the improvement in this PR, of course unless you're up for it, in which case please let me know.
Anyway, I'm going to capture what should be done here:
- In pre-run, try parsing the arg as a repo name.
- If it's not, just leave
opts.Repounassigned (i.e.nil).
- If it's not, just leave
- In run:
- Check current directory is a git (sub)dir.
- If it's not, bail out.
- if
opts.Repoisnil, try parsing the arg as a remote.- if it's not, bail out with an error like "given arg is not a valid repo or git remote".
- Check current directory is a git (sub)dir.
This change will require updating the tests, but has the merit of making the pre-run stage purely static/offline.
|
Fixed the redundant Remotes nil check (factory always sets it) and adjusted tests to always provide a Remotes func. Pushed to fix-9149-set-default-remote-name. |
majiayu000
left a comment
There was a problem hiding this comment.
Addressed: removed redundant Remotes nil check; tests now always supply Remotes func. Ready for re-review.
Summary
When specifying a repository for
gh repo set-default, users can now use a git remote name (e.g., "origin", "upstream") instead of the full OWNER/REPO format.Changes
Example
Test plan
Fixes #9149