Skip to content

Commit 0d0c5ec

Browse files
julieqiujulieqiu
authored
refactor(internal/github): remove error returns from NewClient (#2164)
1 parent 64268c1 commit 0d0c5ec

File tree

4 files changed

+28
-71
lines changed

4 files changed

+28
-71
lines changed

‎internal/automation/trigger.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,7 @@ func RunCommand(ctx context.Context, command string, projectId string, push bool
7373
wrappedClient := &wrappedCloudBuildClient{
7474
client: c,
7575
}
76-
ghClient, err := github.NewClient(os.Getenv("LIBRARIAN_GITHUB_TOKEN"), &github.Repository{})
77-
if err != nil {
78-
return fmt.Errorf("error creating github client: %w", err)
79-
}
76+
ghClient := github.NewClient(os.Getenv("LIBRARIAN_GITHUB_TOKEN"), nil)
8077
return runCommandWithClient(ctx, wrappedClient, ghClient, command, projectId, push, build, forceRun, time.Now())
8178
}
8279

‎internal/github/github.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,16 @@ type Client struct {
5252
}
5353

5454
// NewClient creates a new Client to interact with GitHub.
55-
func NewClient(accessToken string, repo *Repository) (*Client, error) {
55+
func NewClient(accessToken string, repo *Repository) *Client {
5656
return newClientWithHTTP(accessToken, repo, nil)
5757
}
5858

59-
func newClientWithHTTP(accessToken string, repo *Repository, httpClient *http.Client) (*Client, error) {
59+
func newClientWithHTTP(accessToken string, repo *Repository, httpClient *http.Client) *Client {
6060
return &Client{
6161
Client: github.NewClient(httpClient).WithAuthToken(accessToken),
6262
accessToken: accessToken,
6363
repo: repo,
64-
}, nil
64+
}
6565
}
6666

6767
// Token returns the access token for Client.

‎internal/github/github_test.go‎

Lines changed: 23 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package github
1616

1717
import (
18-
"context"
1918
"encoding/json"
2019
"fmt"
2120
"net/http"
@@ -36,10 +35,7 @@ func TestToken(t *testing.T) {
3635
t.Parallel()
3736
want := "fake-token"
3837
repo := &Repository{Owner: "owner", Name: "repo"}
39-
client, err := NewClient(want, repo)
40-
if err != nil {
41-
t.Fatalf("NewClient() error = %v", err)
42-
}
38+
client := NewClient(want, repo)
4339
if got := client.Token(); got != want {
4440
t.Errorf("Token() = %q, want %q", got, want)
4541
}
@@ -102,13 +98,10 @@ func TestGetRawContent(t *testing.T) {
10298
defer server.Close()
10399

104100
repo := &Repository{Owner: "owner", Name: "repo"}
105-
client, err := newClientWithHTTP("fake-token", repo, server.Client())
106-
if err != nil {
107-
t.Fatalf("newClientWithHTTP() error = %v", err)
108-
}
101+
client := newClientWithHTTP("fake-token", repo, server.Client())
109102

110103
client.BaseURL, _ = url.Parse(server.URL + "/")
111-
content, err := client.GetRawContent(context.Background(), "path/to/file", "main")
104+
content, err := client.GetRawContent(t.Context(), "path/to/file", "main")
112105

113106
if test.wantErr {
114107
if err == nil {
@@ -440,13 +433,10 @@ func TestCreatePullRequest(t *testing.T) {
440433
defer server.Close()
441434

442435
repo := &Repository{Owner: "owner", Name: "repo"}
443-
client, err := newClientWithHTTP("fake-token", repo, server.Client())
444-
if err != nil {
445-
t.Fatalf("newClientWithHTTP() error = %v", err)
446-
}
436+
client := newClientWithHTTP("fake-token", repo, server.Client())
447437
client.BaseURL, _ = url.Parse(server.URL + "/")
448438

449-
metadata, err := client.CreatePullRequest(context.Background(), repo, test.remoteBranch, test.remoteBase, test.title, test.body)
439+
metadata, err := client.CreatePullRequest(t.Context(), repo, test.remoteBranch, test.remoteBase, test.title, test.body)
450440

451441
if test.wantErr {
452442
if err == nil {
@@ -510,13 +500,10 @@ func TestAddLabelsToIssue(t *testing.T) {
510500
defer server.Close()
511501

512502
repo := &Repository{Owner: "owner", Name: "repo"}
513-
client, err := newClientWithHTTP("fake-token", repo, server.Client())
514-
if err != nil {
515-
t.Fatalf("newClientWithHTTP() error = %v", err)
516-
}
503+
client := newClientWithHTTP("fake-token", repo, server.Client())
517504
client.BaseURL, _ = url.Parse(server.URL + "/")
518505

519-
err = client.AddLabelsToIssue(context.Background(), repo, test.issueNum, test.labels)
506+
err := client.AddLabelsToIssue(t.Context(), repo, test.issueNum, test.labels)
520507

521508
if test.wantErr {
522509
if err == nil {
@@ -588,13 +575,10 @@ func TestGetLabels(t *testing.T) {
588575
defer server.Close()
589576

590577
repo := &Repository{Owner: "owner", Name: "repo"}
591-
client, err := newClientWithHTTP("fake-token", repo, server.Client())
592-
if err != nil {
593-
t.Fatalf("newClientWithHTTP() error = %v", err)
594-
}
578+
client := newClientWithHTTP("fake-token", repo, server.Client())
595579
client.BaseURL, _ = url.Parse(server.URL + "/")
596580

597-
gotLabels, err := client.GetLabels(context.Background(), test.issueNum)
581+
gotLabels, err := client.GetLabels(t.Context(), test.issueNum)
598582

599583
if test.wantErr {
600584
if err == nil {
@@ -664,13 +648,10 @@ func TestReplaceLabels(t *testing.T) {
664648
defer server.Close()
665649

666650
repo := &Repository{Owner: "owner", Name: "repo"}
667-
client, err := newClientWithHTTP("fake-token", repo, server.Client())
668-
if err != nil {
669-
t.Fatalf("newClientWithHTTP() error = %v", err)
670-
}
651+
client := newClientWithHTTP("fake-token", repo, server.Client())
671652
client.BaseURL, _ = url.Parse(server.URL + "/")
672653

673-
err = client.ReplaceLabels(context.Background(), test.issueNum, test.labels)
654+
err := client.ReplaceLabels(t.Context(), test.issueNum, test.labels)
674655

675656
if test.wantErr {
676657
if err == nil {
@@ -773,13 +754,10 @@ func TestSearchPullRequests(t *testing.T) {
773754
defer server.Close()
774755

775756
repo := &Repository{Owner: "owner", Name: "repo"}
776-
client, err := newClientWithHTTP("fake-token", repo, server.Client())
777-
if err != nil {
778-
t.Fatalf("newClientWithHTTP() error = %v", err)
779-
}
757+
client := newClientWithHTTP("fake-token", repo, server.Client())
780758
client.BaseURL, _ = url.Parse(server.URL + "/")
781759

782-
prs, err := client.SearchPullRequests(context.Background(), test.query)
760+
prs, err := client.SearchPullRequests(t.Context(), test.query)
783761

784762
if test.wantErr {
785763
if err == nil {
@@ -839,13 +817,10 @@ func TestGetPullRequest(t *testing.T) {
839817
defer server.Close()
840818

841819
repo := &Repository{Owner: "owner", Name: "repo"}
842-
client, err := newClientWithHTTP("fake-token", repo, server.Client())
843-
if err != nil {
844-
t.Fatalf("newClientWithHTTP() error = %v", err)
845-
}
820+
client := newClientWithHTTP("fake-token", repo, server.Client())
846821
client.BaseURL, _ = url.Parse(server.URL + "/")
847822

848-
pr, err := client.GetPullRequest(context.Background(), test.number)
823+
pr, err := client.GetPullRequest(t.Context(), test.number)
849824

850825
if test.wantErr {
851826
if err == nil {
@@ -919,13 +894,10 @@ func TestCreateRelease(t *testing.T) {
919894
defer server.Close()
920895

921896
repo := &Repository{Owner: "owner", Name: "repo"}
922-
client, err := newClientWithHTTP("fake-token", repo, server.Client())
923-
if err != nil {
924-
t.Fatalf("newClientWithHTTP() error = %v", err)
925-
}
897+
client := newClientWithHTTP("fake-token", repo, server.Client())
926898
client.BaseURL, _ = url.Parse(server.URL + "/")
927899

928-
release, err := client.CreateRelease(context.Background(), test.tagName, test.releaseName, test.body, test.commitish)
900+
release, err := client.CreateRelease(t.Context(), test.tagName, test.releaseName, test.body, test.commitish)
929901

930902
if test.wantErr {
931903
if err == nil {
@@ -991,13 +963,10 @@ func TestCreateIssueComment(t *testing.T) {
991963
defer server.Close()
992964

993965
repo := &Repository{Owner: "owner", Name: "repo"}
994-
client, err := newClientWithHTTP("fake-token", repo, server.Client())
995-
if err != nil {
996-
t.Fatalf("newClientWithHTTP() error = %v", err)
997-
}
966+
client := newClientWithHTTP("fake-token", repo, server.Client())
998967
client.BaseURL, _ = url.Parse(server.URL + "/")
999968

1000-
err = client.CreateIssueComment(context.Background(), test.number, test.body)
969+
err := client.CreateIssueComment(t.Context(), test.number, test.body)
1001970

1002971
if test.wantErr {
1003972
if err == nil {
@@ -1057,13 +1026,10 @@ func TestFindMergedPullRequestsWithPendingReleaseLabel(t *testing.T) {
10571026
defer server.Close()
10581027

10591028
repo := &Repository{Owner: "owner", Name: "repo"}
1060-
client, err := newClientWithHTTP("fake-token", repo, server.Client())
1061-
if err != nil {
1062-
t.Fatalf("newClientWithHTTP() error = %v", err)
1063-
}
1029+
client := newClientWithHTTP("fake-token", repo, server.Client())
10641030
client.BaseURL, _ = url.Parse(server.URL + "/")
10651031

1066-
prs, err := client.FindMergedPullRequestsWithPendingReleaseLabel(context.Background(), "owner", "repo")
1032+
prs, err := client.FindMergedPullRequestsWithPendingReleaseLabel(t.Context(), "owner", "repo")
10671033

10681034
if test.wantErr {
10691035
if err == nil {
@@ -1127,13 +1093,10 @@ func TestCreateTag(t *testing.T) {
11271093
defer server.Close()
11281094

11291095
repo := &Repository{Owner: "owner", Name: "repo"}
1130-
client, err := newClientWithHTTP("fake-token", repo, server.Client())
1131-
if err != nil {
1132-
t.Fatalf("newClientWithHTTP() error = %v", err)
1133-
}
1096+
client := newClientWithHTTP("fake-token", repo, server.Client())
11341097
client.BaseURL, _ = url.Parse(server.URL + "/")
11351098

1136-
err = client.CreateTag(context.Background(), test.tagName, test.commitSHA)
1099+
err := client.CreateTag(t.Context(), test.tagName, test.commitSHA)
11371100

11381101
if test.wantErr {
11391102
if err == nil {

‎internal/librarian/command.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,7 @@ func newCommandRunner(cfg *config.Config) (*commandRunner, error) {
139139
return nil, fmt.Errorf("failed to get GitHub repo from remote: %w", err)
140140
}
141141
}
142-
ghClient, err := github.NewClient(cfg.GitHubToken, gitRepo)
143-
if err != nil {
144-
return nil, fmt.Errorf("failed to create GitHub client: %w", err)
145-
}
142+
ghClient := github.NewClient(cfg.GitHubToken, gitRepo)
146143

147144
container, err := docker.New(cfg.WorkRoot, image, cfg.UserUID, cfg.UserGID)
148145
if err != nil {

0 commit comments

Comments
 (0)