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
Use go-gh config package #5771
Use go-gh config package #5771
Conversation
484ecd5
to
1379d17
Compare
ghrepo.SetDefaultHost(host) | ||
} | ||
host, _ := cfg.DefaultHost() | ||
ghrepo.SetDefaultHost(host) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After this change we will be able to remove this and use ghAuth.DefaultHost()
inside of ghrepo
instead of the defaultHostOverride
variable.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
1e0d995
to
e64b4f4
Compare
e64b4f4
to
35b8c35
Compare
35b8c35
to
ed43b78
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the giant body of work!
This diff could have been potentially made smaller by keeping some old interfaces and their return values the same: the Aliases API, for example, cfg.Set
, and DefaultHost. If something used to return an error before but doesn't anymore, it would have been fine to keep returning a nil error (for example). But now that you've already done the legwork to rewrite all the call points, I guess that's fine!
This PR changes all uses of
config
to now use thego-gh
config
package. The majority of non-test file changes are just modifying the return values fromcfg.DefaultHost()
, since it no longer returns an error.Follow up to cli/go-gh#44 and cli/go-gh#45.
cc #5560