Skip to content

Post go-gh integration cleanup#5933

Merged
samcoe merged 1 commit intotrunkfrom
go-gh-cleanup
Jul 13, 2022
Merged

Post go-gh integration cleanup#5933
samcoe merged 1 commit intotrunkfrom
go-gh-cleanup

Conversation

@samcoe
Copy link
Copy Markdown
Contributor

@samcoe samcoe commented Jul 13, 2022 •
edited
Loading

This is a small refactoring PR now that the go-gh integration has been shipped. It does two things:

  1. Remove the now unused Get function from the configGetter interface and rename it to tokenGetter.
  2. Remove ghrepo.SetDefaultHost and refactor implementation of ghrepo.defaultHost to use ghAuth.DefaultHost instead. Functionally this is equivalent but it removes defaultHostOverride, global state, from the ghrepo package.

cc #5560

@samcoe samcoe self-assigned this Jul 13, 2022
samcoe
samcoe commented Jul 13, 2022
View reviewed changes
Comment thread api/http_client.go

type configGetter interface {
Get(string, string) (string, error)
type tokenGetter interface {
Copy link
Copy Markdown
Contributor Author

@samcoe samcoe Jul 13, 2022

Choose a reason for hiding this comment

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

Open to better names for this interface.

@samcoe samcoe marked this pull request as ready for review July 13, 2022 12:27
@samcoe samcoe requested a review from a team as a code owner July 13, 2022 12:27
@samcoe samcoe requested review from vilmibm and removed request for a team July 13, 2022 12:27
@samcoe samcoe merged commit e5cf302 into trunk Jul 13, 2022
@samcoe samcoe deleted the go-gh-cleanup branch July 13, 2022 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vilmibm vilmibm Awaiting requested review from vilmibm vilmibm is a code owner automatically assigned from cli/code-reviewers

1 more reviewer

@mislav mislav mislav approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@samcoe samcoe

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@samcoe @mislav @cliAutomation