Skip to content

Commit 793450f

Browse files
rupokghoshrupokghosh
committed
refactor: convert required pointer params to values
update exmaples change to using values refactor(git)!: improve CreateRef API with value types and exported struct format
1 parent 6dda213 commit 793450f

9 files changed

Lines changed: 65 additions & 105 deletions

File tree

‎example/commitpr/main.go‎

Lines changed: 3 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,7 @@ 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, false)
173173
return err
174174
}
175175

‎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 string, repo string, blob *Blob) (*Blob, *Response, error) {
74+
func (s *GitService) CreateBlob(ctx context.Context, owner string, 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 string, 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 string, 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 <go-github@github.com> 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: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ 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

4646
// updateRefRequest represents the payload for updating a reference.
@@ -127,20 +127,22 @@ 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 string, 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 string, 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+
// back-compat with previous behavior that didn't require 'refs/' prefix
140+
if !strings.HasPrefix(ref.Ref, "refs/") {
141+
ref.Ref = "refs/" + ref.Ref
142+
}
143+
138144
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-
})
145+
req, err := s.client.NewRequest("POST", u, ref)
144146
if err != nil {
145147
return nil, nil, err
146148
}
@@ -159,10 +161,7 @@ func (s *GitService) CreateRef(ctx context.Context, owner string, repo string, r
159161
// GitHub API docs: https://docs.github.com/rest/git/refs#update-a-reference
160162
//
161163
//meta:operation PATCH /repos/{owner}/{repo}/git/refs/{ref}
162-
func (s *GitService) UpdateRef(ctx context.Context, owner string, repo string, ref *Reference, force bool) (*Reference, *Response, error) {
163-
if ref == nil {
164-
return nil, nil, errors.New("reference must be provided")
165-
}
164+
func (s *GitService) UpdateRef(ctx context.Context, owner string, repo string, ref Reference, force bool) (*Reference, *Response, error) {
166165
if ref.Ref == nil {
167166
return nil, nil, errors.New("ref must be provided")
168167
}

‎github/git_refs_test.go‎

Lines changed: 30 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -386,18 +386,18 @@ func TestGitService_CreateRef(t *testing.T) {
386386
t.Parallel()
387387
client, mux, _ := setup(t)
388388

389-
args := &createRefRequest{
390-
Ref: Ptr("refs/heads/b"),
391-
SHA: Ptr("aa218f56b14c9653891f9e74264a383fa43fefbd"),
389+
args := CreateRef{
390+
Ref: "refs/heads/b",
391+
SHA: "aa218f56b14c9653891f9e74264a383fa43fefbd",
392392
}
393393

394394
mux.HandleFunc("/repos/o/r/git/refs", func(w http.ResponseWriter, r *http.Request) {
395-
v := new(createRefRequest)
395+
v := new(CreateRef)
396396
assertNilError(t, json.NewDecoder(r.Body).Decode(v))
397397

398398
testMethod(t, r, "POST")
399-
if !cmp.Equal(v, args) {
400-
t.Errorf("Request body = %+v, want %+v", v, args)
399+
if !cmp.Equal(*v, args) {
400+
t.Errorf("Request body = %+v, want %+v", *v, args)
401401
}
402402
fmt.Fprint(w, `
403403
{
@@ -412,11 +412,9 @@ func TestGitService_CreateRef(t *testing.T) {
412412
})
413413

414414
ctx := context.Background()
415-
ref, _, err := client.Git.CreateRef(ctx, "o", "r", &Reference{
416-
Ref: Ptr("refs/heads/b"),
417-
Object: &GitObject{
418-
SHA: Ptr("aa218f56b14c9653891f9e74264a383fa43fefbd"),
419-
},
415+
ref, _, err := client.Git.CreateRef(ctx, "o", "r", CreateRef{
416+
Ref: "heads/b",
417+
SHA: "aa218f56b14c9653891f9e74264a383fa43fefbd",
420418
})
421419
if err != nil {
422420
t.Errorf("Git.CreateRef returned error: %v", err)
@@ -436,41 +434,31 @@ func TestGitService_CreateRef(t *testing.T) {
436434
}
437435

438436
// without 'refs/' prefix
439-
_, _, err = client.Git.CreateRef(ctx, "o", "r", &Reference{
440-
Ref: Ptr("heads/b"),
441-
Object: &GitObject{
442-
SHA: Ptr("aa218f56b14c9653891f9e74264a383fa43fefbd"),
443-
},
437+
_, _, err = client.Git.CreateRef(ctx, "o", "r", CreateRef{
438+
Ref: "heads/b",
439+
SHA: "aa218f56b14c9653891f9e74264a383fa43fefbd",
444440
})
445441
if err != nil {
446442
t.Errorf("Git.CreateRef returned error: %v", err)
447443
}
448444

449445
const methodName = "CreateRef"
450446
testBadOptions(t, methodName, func() (err error) {
451-
_, _, err = client.Git.CreateRef(ctx, "o", "r", nil)
447+
_, _, err = client.Git.CreateRef(ctx, "o", "r", CreateRef{Ref: ""})
452448
return err
453449
})
454450
testBadOptions(t, methodName, func() (err error) {
455-
_, _, err = client.Git.CreateRef(ctx, "o", "r", &Reference{Ref: nil})
456-
return err
457-
})
458-
testBadOptions(t, methodName, func() (err error) {
459-
_, _, err = client.Git.CreateRef(ctx, "\n", "\n", &Reference{
460-
Ref: Ptr("refs/heads/b"),
461-
Object: &GitObject{
462-
SHA: Ptr("aa218f56b14c9653891f9e74264a383fa43fefbd"),
463-
},
451+
_, _, err = client.Git.CreateRef(ctx, "\n", "\n", CreateRef{
452+
Ref: "refs/heads/b",
453+
SHA: "aa218f56b14c9653891f9e74264a383fa43fefbd",
464454
})
465455
return err
466456
})
467457

468458
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
469-
got, resp, err := client.Git.CreateRef(ctx, "o", "r", &Reference{
470-
Ref: Ptr("refs/heads/b"),
471-
Object: &GitObject{
472-
SHA: Ptr("aa218f56b14c9653891f9e74264a383fa43fefbd"),
473-
},
459+
got, resp, err := client.Git.CreateRef(ctx, "o", "r", CreateRef{
460+
Ref: "refs/heads/b",
461+
SHA: "aa218f56b14c9653891f9e74264a383fa43fefbd",
474462
})
475463
if got != nil {
476464
t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
@@ -509,7 +497,7 @@ func TestGitService_UpdateRef(t *testing.T) {
509497
})
510498

511499
ctx := context.Background()
512-
ref, _, err := client.Git.UpdateRef(ctx, "o", "r", &Reference{
500+
ref, _, err := client.Git.UpdateRef(ctx, "o", "r", Reference{
513501
Ref: Ptr("refs/heads/b"),
514502
Object: &GitObject{SHA: Ptr("aa218f56b14c9653891f9e74264a383fa43fefbd")},
515503
}, true)
@@ -531,7 +519,7 @@ func TestGitService_UpdateRef(t *testing.T) {
531519
}
532520

533521
// without 'refs/' prefix
534-
_, _, err = client.Git.UpdateRef(ctx, "o", "r", &Reference{
522+
_, _, err = client.Git.UpdateRef(ctx, "o", "r", Reference{
535523
Ref: Ptr("heads/b"),
536524
Object: &GitObject{SHA: Ptr("aa218f56b14c9653891f9e74264a383fa43fefbd")},
537525
}, true)
@@ -541,23 +529,19 @@ func TestGitService_UpdateRef(t *testing.T) {
541529

542530
const methodName = "UpdateRef"
543531
testBadOptions(t, methodName, func() (err error) {
544-
_, _, err = client.Git.UpdateRef(ctx, "o", "r", nil, true)
532+
_, _, err = client.Git.UpdateRef(ctx, "o", "r", Reference{Ref: nil}, true)
545533
return err
546534
})
547535
testBadOptions(t, methodName, func() (err error) {
548-
_, _, err = client.Git.UpdateRef(ctx, "o", "r", &Reference{Ref: nil}, true)
549-
return err
550-
})
551-
testBadOptions(t, methodName, func() (err error) {
552-
_, _, err = client.Git.UpdateRef(ctx, "\n", "\n", &Reference{
536+
_, _, err = client.Git.UpdateRef(ctx, "\n", "\n", Reference{
553537
Ref: Ptr("refs/heads/b"),
554538
Object: &GitObject{SHA: Ptr("aa218f56b14c9653891f9e74264a383fa43fefbd")},
555539
}, true)
556540
return err
557541
})
558542

559543
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
560-
got, resp, err := client.Git.UpdateRef(ctx, "o", "r", &Reference{
544+
got, resp, err := client.Git.UpdateRef(ctx, "o", "r", Reference{
561545
Ref: Ptr("refs/heads/b"),
562546
Object: &GitObject{SHA: Ptr("aa218f56b14c9653891f9e74264a383fa43fefbd")},
563547
}, true)
@@ -670,7 +654,7 @@ func TestGitService_UpdateRef_pathEscape(t *testing.T) {
670654
})
671655

672656
ctx := context.Background()
673-
ref, _, err := client.Git.UpdateRef(ctx, "o", "r", &Reference{
657+
ref, _, err := client.Git.UpdateRef(ctx, "o", "r", Reference{
674658
Ref: Ptr("refs/heads/b#1"),
675659
Object: &GitObject{SHA: Ptr("aa218f56b14c9653891f9e74264a383fa43fefbd")},
676660
}, true)
@@ -740,13 +724,13 @@ func TestGitObject_Marshal(t *testing.T) {
740724
testJSONMarshal(t, u, want)
741725
}
742726

743-
func TestCreateRefRequest_Marshal(t *testing.T) {
727+
func TestCreateRef_Marshal(t *testing.T) {
744728
t.Parallel()
745-
testJSONMarshal(t, &createRefRequest{}, "{}")
729+
testJSONMarshal(t, CreateRef{}, `{"ref":"","sha":""}`)
746730

747-
u := &createRefRequest{
748-
Ref: Ptr("ref"),
749-
SHA: Ptr("sha"),
731+
u := CreateRef{
732+
Ref: "ref",
733+
SHA: "sha",
750734
}
751735

752736
want := `{

‎github/git_tags.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package github
77

88
import (
99
"context"
10-
"errors"
1110
"fmt"
1211
)
1312

@@ -60,11 +59,7 @@ func (s *GitService) GetTag(ctx context.Context, owner string, repo string, sha
6059
// GitHub API docs: https://docs.github.com/rest/git/tags#create-a-tag-object
6160
//
6261
//meta:operation POST /repos/{owner}/{repo}/git/tags
63-
func (s *GitService) CreateTag(ctx context.Context, owner string, repo string, tag *Tag) (*Tag, *Response, error) {
64-
if tag == nil {
65-
return nil, nil, errors.New("tag must be provided")
66-
}
67-
62+
func (s *GitService) CreateTag(ctx context.Context, owner string, repo string, tag Tag) (*Tag, *Response, error) {
6863
u := fmt.Sprintf("repos/%v/%v/git/tags", owner, repo)
6964

7065
// convert Tag into a createTagRequest

0 commit comments

Comments
 (0)