Skip to content

Commit db8be8b

Browse files
rupokghoshrupokghosh
authored
refactor!: Change GitService methods to pass required params by-value instead of by-ref (#3654)
BREAKING CHANGE: `GitService` methods now pass required params by-value instead of by-ref.
1 parent 17f7ee4 commit db8be8b

File tree

11 files changed

+164
-175
lines changed

11 files changed

+164
-175
lines changed

‎example/commitpr/main.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func getRef() (ref *github.Reference, err error) {
8181
if baseRef, _, err = client.Git.GetRef(ctx, *sourceOwner, *sourceRepo, branchRef(*baseBranch)); err != nil {
8282
return nil, err
8383
}
84-
newRef := &github.Reference{Ref: github.Ptr(branchRef(*commitBranch)), Object: &github.GitObject{SHA: baseRef.Object.SHA}}
84+
newRef := github.CreateRef{Ref: branchRef(*commitBranch), SHA: *baseRef.Object.SHA}
8585
ref, _, err = client.Git.CreateRef(ctx, *sourceOwner, *sourceRepo, newRef)
8686
return ref, err
8787
}
@@ -143,7 +143,7 @@ func pushCommit(ref *github.Reference, tree *github.Tree) (err error) {
143143
// Create the commit using the tree.
144144
date := time.Now()
145145
author := &github.CommitAuthor{Date: &github.Timestamp{Time: date}, Name: authorName, Email: authorEmail}
146-
commit := &github.Commit{Author: author, Message: commitMessage, Tree: tree, Parents: []*github.Commit{parent.Commit}}
146+
commit := github.Commit{Author: author, Message: commitMessage, Tree: tree, Parents: []*github.Commit{parent.Commit}}
147147
opts := github.CreateCommitOptions{}
148148
if *privateKey != "" {
149149
armoredBlock, e := os.ReadFile(*privateKey)
@@ -169,7 +169,10 @@ func pushCommit(ref *github.Reference, tree *github.Tree) (err error) {
169169

170170
// Attach the commit to the master branch.
171171
ref.Object.SHA = newCommit.SHA
172-
_, _, err = client.Git.UpdateRef(ctx, *sourceOwner, *sourceRepo, ref, false)
172+
_, _, err = client.Git.UpdateRef(ctx, *sourceOwner, *sourceRepo, *ref.Ref, github.UpdateRef{
173+
SHA: *newCommit.SHA,
174+
Force: github.Ptr(false),
175+
})
173176
return err
174177
}
175178

‎github/git_blobs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func (s *GitService) GetBlobRaw(ctx context.Context, owner, repo, sha string) ([
7171
// GitHub API docs: https://docs.github.com/rest/git/blobs#create-a-blob
7272
//
7373
//meta:operation POST /repos/{owner}/{repo}/git/blobs
74-
func (s *GitService) CreateBlob(ctx context.Context, owner, repo string, blob *Blob) (*Blob, *Response, error) {
74+
func (s *GitService) CreateBlob(ctx context.Context, owner, repo string, blob Blob) (*Blob, *Response, error) {
7575
u := fmt.Sprintf("repos/%v/%v/git/blobs", owner, repo)
7676
req, err := s.client.NewRequest("POST", u, blob)
7777
if err != nil {

‎github/git_blobs_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func TestGitService_CreateBlob(t *testing.T) {
109109
t.Parallel()
110110
client, mux, _ := setup(t)
111111

112-
input := &Blob{
112+
input := Blob{
113113
SHA: Ptr("s"),
114114
Content: Ptr("blob content"),
115115
Encoding: Ptr("utf-8"),
@@ -123,8 +123,8 @@ func TestGitService_CreateBlob(t *testing.T) {
123123
testMethod(t, r, "POST")
124124

125125
want := input
126-
if !cmp.Equal(v, want) {
127-
t.Errorf("Git.CreateBlob request body: %+v, want %+v", v, want)
126+
if !cmp.Equal(*v, want) {
127+
t.Errorf("Git.CreateBlob request body: %+v, want %+v", *v, want)
128128
}
129129

130130
fmt.Fprint(w, `{
@@ -143,8 +143,8 @@ func TestGitService_CreateBlob(t *testing.T) {
143143

144144
want := input
145145

146-
if !cmp.Equal(*blob, *want) {
147-
t.Errorf("Git.CreateBlob returned %+v, want %+v", *blob, *want)
146+
if !cmp.Equal(*blob, want) {
147+
t.Errorf("Git.CreateBlob returned %+v, want %+v", *blob, want)
148148
}
149149

150150
const methodName = "CreateBlob"
@@ -167,7 +167,7 @@ func TestGitService_CreateBlob_invalidOwner(t *testing.T) {
167167
client, _, _ := setup(t)
168168

169169
ctx := context.Background()
170-
_, _, err := client.Git.CreateBlob(ctx, "%", "%", &Blob{})
170+
_, _, err := client.Git.CreateBlob(ctx, "%", "%", Blob{})
171171
testURLParseError(t, err)
172172
}
173173

‎github/git_commits.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,7 @@ type CreateCommitOptions struct {
126126
// GitHub API docs: https://docs.github.com/rest/git/commits#create-a-commit
127127
//
128128
//meta:operation POST /repos/{owner}/{repo}/git/commits
129-
func (s *GitService) CreateCommit(ctx context.Context, owner, repo string, commit *Commit, opts *CreateCommitOptions) (*Commit, *Response, error) {
130-
if commit == nil {
131-
return nil, nil, errors.New("commit must be provided")
132-
}
129+
func (s *GitService) CreateCommit(ctx context.Context, owner, repo string, commit Commit, opts *CreateCommitOptions) (*Commit, *Response, error) {
133130
if opts == nil {
134131
opts = &CreateCommitOptions{}
135132
}

‎github/git_commits_test.go

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ func TestGitService_CreateCommit(t *testing.T) {
176176
t.Parallel()
177177
client, mux, _ := setup(t)
178178

179-
input := &Commit{
179+
input := Commit{
180180
Message: Ptr("Commit Message."),
181181
Tree: &Tree{SHA: Ptr("t")},
182182
Parents: []*Commit{{SHA: Ptr("p")}},
@@ -231,7 +231,7 @@ func TestGitService_CreateSignedCommit(t *testing.T) {
231231

232232
signature := "----- BEGIN PGP SIGNATURE -----\n\naaaa\naaaa\n----- END PGP SIGNATURE -----"
233233

234-
input := &Commit{
234+
input := Commit{
235235
Message: Ptr("Commit Message."),
236236
Tree: &Tree{SHA: Ptr("t")},
237237
Parents: []*Commit{{SHA: Ptr("p")}},
@@ -288,7 +288,7 @@ func TestGitService_CreateSignedCommitWithInvalidParams(t *testing.T) {
288288
t.Parallel()
289289
client, _, _ := setup(t)
290290

291-
input := &Commit{}
291+
input := Commit{}
292292

293293
ctx := context.Background()
294294
opts := CreateCommitOptions{Signer: uncalledSigner(t)}
@@ -298,17 +298,6 @@ func TestGitService_CreateSignedCommitWithInvalidParams(t *testing.T) {
298298
}
299299
}
300300

301-
func TestGitService_CreateCommitWithNilCommit(t *testing.T) {
302-
t.Parallel()
303-
client, _, _ := setup(t)
304-
305-
ctx := context.Background()
306-
_, _, err := client.Git.CreateCommit(ctx, "o", "r", nil, nil)
307-
if err == nil {
308-
t.Error("Expected error to be returned because commit=nil")
309-
}
310-
}
311-
312301
func TestGitService_CreateCommit_WithSigner(t *testing.T) {
313302
t.Parallel()
314303
client, mux, _ := setup(t)
@@ -327,7 +316,7 @@ committer go-github <[email protected]> 1493849023 +0200
327316
328317
Commit Message.`
329318
sha := "commitSha"
330-
input := &Commit{
319+
input := Commit{
331320
SHA: &sha,
332321
Message: Ptr("Commit Message."),
333322
Tree: &Tree{SHA: Ptr("t")},
@@ -513,7 +502,7 @@ func TestGitService_CreateCommit_invalidOwner(t *testing.T) {
513502
client, _, _ := setup(t)
514503

515504
ctx := context.Background()
516-
_, _, err := client.Git.CreateCommit(ctx, "%", "%", &Commit{}, nil)
505+
_, _, err := client.Git.CreateCommit(ctx, "%", "%", Commit{}, nil)
517506
testURLParseError(t, err)
518507
}
519508

‎github/git_refs.go

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,16 @@ func (o GitObject) String() string {
3737
return Stringify(o)
3838
}
3939

40-
// createRefRequest represents the payload for creating a reference.
41-
type createRefRequest struct {
42-
Ref *string `json:"ref"`
43-
SHA *string `json:"sha"`
40+
// CreateRef represents the payload for creating a reference.
41+
type CreateRef struct {
42+
Ref string `json:"ref"`
43+
SHA string `json:"sha"`
4444
}
4545

46-
// updateRefRequest represents the payload for updating a reference.
47-
type updateRefRequest struct {
48-
SHA *string `json:"sha"`
49-
Force *bool `json:"force"`
46+
// UpdateRef represents the payload for updating a reference.
47+
type UpdateRef struct {
48+
SHA string `json:"sha"`
49+
Force *bool `json:"force,omitempty"`
5050
}
5151

5252
// GetRef fetches a single reference in a repository.
@@ -127,20 +127,20 @@ func (s *GitService) ListMatchingRefs(ctx context.Context, owner, repo string, o
127127
// GitHub API docs: https://docs.github.com/rest/git/refs#create-a-reference
128128
//
129129
//meta:operation POST /repos/{owner}/{repo}/git/refs
130-
func (s *GitService) CreateRef(ctx context.Context, owner, repo string, ref *Reference) (*Reference, *Response, error) {
131-
if ref == nil {
132-
return nil, nil, errors.New("reference must be provided")
133-
}
134-
if ref.Ref == nil {
130+
func (s *GitService) CreateRef(ctx context.Context, owner, repo string, ref CreateRef) (*Reference, *Response, error) {
131+
if ref.Ref == "" {
135132
return nil, nil, errors.New("ref must be provided")
136133
}
137134

135+
if ref.SHA == "" {
136+
return nil, nil, errors.New("sha must be provided")
137+
}
138+
139+
// ensure the 'refs/' prefix is present
140+
ref.Ref = "refs/" + strings.TrimPrefix(ref.Ref, "refs/")
141+
138142
u := fmt.Sprintf("repos/%v/%v/git/refs", owner, repo)
139-
req, err := s.client.NewRequest("POST", u, &createRefRequest{
140-
// back-compat with previous behavior that didn't require 'refs/' prefix
141-
Ref: Ptr("refs/" + strings.TrimPrefix(*ref.Ref, "refs/")),
142-
SHA: ref.Object.SHA,
143-
})
143+
req, err := s.client.NewRequest("POST", u, ref)
144144
if err != nil {
145145
return nil, nil, err
146146
}
@@ -159,20 +159,18 @@ func (s *GitService) CreateRef(ctx context.Context, owner, repo string, ref *Ref
159159
// GitHub API docs: https://docs.github.com/rest/git/refs#update-a-reference
160160
//
161161
//meta:operation PATCH /repos/{owner}/{repo}/git/refs/{ref}
162-
func (s *GitService) UpdateRef(ctx context.Context, owner, repo string, ref *Reference, force bool) (*Reference, *Response, error) {
163-
if ref == nil {
164-
return nil, nil, errors.New("reference must be provided")
165-
}
166-
if ref.Ref == nil {
162+
func (s *GitService) UpdateRef(ctx context.Context, owner, repo, ref string, updateRef UpdateRef) (*Reference, *Response, error) {
163+
if ref == "" {
167164
return nil, nil, errors.New("ref must be provided")
168165
}
169166

170-
refPath := strings.TrimPrefix(*ref.Ref, "refs/")
167+
if updateRef.SHA == "" {
168+
return nil, nil, errors.New("sha must be provided")
169+
}
170+
171+
refPath := strings.TrimPrefix(ref, "refs/")
171172
u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, refURLEscape(refPath))
172-
req, err := s.client.NewRequest("PATCH", u, &updateRefRequest{
173-
SHA: ref.Object.SHA,
174-
Force: &force,
175-
})
173+
req, err := s.client.NewRequest("PATCH", u, updateRef)
176174
if err != nil {
177175
return nil, nil, err
178176
}

0 commit comments

Comments
 (0)