Skip to content

Commit 9b28aa4

Browse files
codyosscodyoss
authored
refactor(internal/gitrepo): define a Remote type and use it (#2373)
We should avoid putting other packages types in APIs of our types. This breaks encapalation of the library we are trying to wrap. This package has more types like this that I will follow up with in future PRs. Updates: #2372
1 parent 2edd323 commit 9b28aa4

File tree

7 files changed

+84
-75
lines changed

7 files changed

+84
-75
lines changed

‎internal/github/github.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,10 +226,9 @@ func FetchGitHubRepoFromRemote(repo gitrepo.Repository) (*Repository, error) {
226226
}
227227

228228
for _, remote := range remotes {
229-
if remote.Config().Name == "origin" {
230-
urls := remote.Config().URLs
231-
if len(urls) > 0 {
232-
return ParseRemote(urls[0])
229+
if remote.Name == "origin" {
230+
if len(remote.URLs) > 0 {
231+
return ParseRemote(remote.URLs[0])
233232
}
234233
}
235234
}

‎internal/gitrepo/gitrepo.go‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ type Repository interface {
3737
AddAll() (git.Status, error)
3838
Commit(msg string) error
3939
IsClean() (bool, error)
40-
Remotes() ([]*git.Remote, error)
40+
Remotes() ([]*Remote, error)
4141
GetDir() string
4242
HeadHash() (string, error)
4343
ChangedFilesInCommit(commitHash string) ([]string, error)
@@ -65,6 +65,12 @@ type Commit struct {
6565
When time.Time
6666
}
6767

68+
// Remote represent a git remote.
69+
type Remote struct {
70+
Name string
71+
URLs []string
72+
}
73+
6874
// RepositoryOptions are used to configure a [LocalRepository].
6975
type RepositoryOptions struct {
7076
// Dir is the directory where the repository will reside locally. Required.
@@ -222,8 +228,17 @@ func (r *LocalRepository) IsClean() (bool, error) {
222228
}
223229

224230
// Remotes returns the remotes within the repository.
225-
func (r *LocalRepository) Remotes() ([]*git.Remote, error) {
226-
return r.repo.Remotes()
231+
func (r *LocalRepository) Remotes() ([]*Remote, error) {
232+
gitRemotes, err := r.repo.Remotes()
233+
if err != nil {
234+
return nil, err
235+
}
236+
var remotes []*Remote
237+
for _, remote := range gitRemotes {
238+
remotes = append(remotes, &Remote{Name: remote.Config().Name, URLs: remote.Config().URLs})
239+
}
240+
241+
return remotes, nil
227242
}
228243

229244
// HeadHash returns hash of the commit for the repository's HEAD.

‎internal/gitrepo/gitrepo_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ func TestRemotes(t *testing.T) {
588588

589589
gotRemotes := make(map[string][]string)
590590
for _, r := range got {
591-
gotRemotes[r.Config().Name] = r.Config().URLs
591+
gotRemotes[r.Name] = r.URLs
592592
}
593593
if diff := cmp.Diff(test.setupRemotes, gotRemotes); diff != "" {
594594
t.Errorf("Remotes() mismatch (-want +got):\n%s", diff)

‎internal/librarian/command_test.go‎

Lines changed: 34 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ import (
2626
"testing"
2727

2828
"github.com/go-git/go-git/v5"
29-
gogitConfig "github.com/go-git/go-git/v5/config"
30-
"github.com/go-git/go-git/v5/storage/memory"
3129
"github.com/google/go-cmp/cmp"
3230
"github.com/googleapis/librarian/internal/config"
3331
"github.com/googleapis/librarian/internal/github"
@@ -1322,16 +1320,16 @@ func TestCommitAndPush(t *testing.T) {
13221320
{
13231321
name: "create a commit",
13241322
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1325-
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1323+
remote := &gitrepo.Remote{
13261324
Name: "origin",
13271325
URLs: []string{"https://github.com/googleapis/librarian.git"},
1328-
})
1326+
}
13291327
status := make(git.Status)
13301328
status["file.txt"] = &git.FileStatus{Worktree: git.Modified}
13311329
return &MockRepository{
13321330
Dir: t.TempDir(),
13331331
AddAllStatus: status,
1334-
RemotesValue: []*git.Remote{remote},
1332+
RemotesValue: []*gitrepo.Remote{remote},
13351333
}
13361334
},
13371335
setupMockClient: func(t *testing.T) GitHubClient {
@@ -1345,16 +1343,16 @@ func TestCommitAndPush(t *testing.T) {
13451343
{
13461344
name: "create a generate pull request",
13471345
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1348-
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1346+
remote := &gitrepo.Remote{
13491347
Name: "origin",
13501348
URLs: []string{"https://github.com/googleapis/librarian.git"},
1351-
})
1349+
}
13521350
status := make(git.Status)
13531351
status["file.txt"] = &git.FileStatus{Worktree: git.Modified}
13541352
return &MockRepository{
13551353
Dir: t.TempDir(),
13561354
AddAllStatus: status,
1357-
RemotesValue: []*git.Remote{remote},
1355+
RemotesValue: []*gitrepo.Remote{remote},
13581356
}
13591357
},
13601358
setupMockClient: func(t *testing.T) GitHubClient {
@@ -1369,16 +1367,16 @@ func TestCommitAndPush(t *testing.T) {
13691367
{
13701368
name: "create a release pull request",
13711369
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1372-
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1370+
remote := &gitrepo.Remote{
13731371
Name: "origin",
13741372
URLs: []string{"https://github.com/googleapis/librarian.git"},
1375-
})
1373+
}
13761374
status := make(git.Status)
13771375
status["file.txt"] = &git.FileStatus{Worktree: git.Modified}
13781376
return &MockRepository{
13791377
Dir: t.TempDir(),
13801378
AddAllStatus: status,
1381-
RemotesValue: []*git.Remote{remote},
1379+
RemotesValue: []*gitrepo.Remote{remote},
13821380
}
13831381
},
13841382
setupMockClient: func(t *testing.T) GitHubClient {
@@ -1398,7 +1396,7 @@ func TestCommitAndPush(t *testing.T) {
13981396
return &MockRepository{
13991397
Dir: t.TempDir(),
14001398
AddAllStatus: status,
1401-
RemotesValue: []*git.Remote{}, // No remotes
1399+
RemotesValue: []*gitrepo.Remote{}, // No remotes
14021400
}
14031401
},
14041402
setupMockClient: func(t *testing.T) GitHubClient {
@@ -1413,13 +1411,13 @@ func TestCommitAndPush(t *testing.T) {
14131411
{
14141412
name: "AddAll error",
14151413
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1416-
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1414+
remote := &gitrepo.Remote{
14171415
Name: "origin",
14181416
URLs: []string{"https://github.com/googleapis/librarian.git"},
1419-
})
1417+
}
14201418
return &MockRepository{
14211419
Dir: t.TempDir(),
1422-
RemotesValue: []*git.Remote{remote},
1420+
RemotesValue: []*gitrepo.Remote{remote},
14231421
AddAllError: errors.New("mock add all error"),
14241422
}
14251423
},
@@ -1434,17 +1432,17 @@ func TestCommitAndPush(t *testing.T) {
14341432
{
14351433
name: "Create branch error",
14361434
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1437-
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1435+
remote := &gitrepo.Remote{
14381436
Name: "origin",
14391437
URLs: []string{"https://github.com/googleapis/librarian.git"},
1440-
})
1438+
}
14411439

14421440
status := make(git.Status)
14431441
status["file.txt"] = &git.FileStatus{Worktree: git.Modified}
14441442
return &MockRepository{
14451443
Dir: t.TempDir(),
14461444
AddAllStatus: status,
1447-
RemotesValue: []*git.Remote{remote},
1445+
RemotesValue: []*gitrepo.Remote{remote},
14481446
CreateBranchAndCheckoutError: errors.New("create branch error"),
14491447
}
14501448
},
@@ -1459,17 +1457,17 @@ func TestCommitAndPush(t *testing.T) {
14591457
{
14601458
name: "Commit error",
14611459
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1462-
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1460+
remote := &gitrepo.Remote{
14631461
Name: "origin",
14641462
URLs: []string{"https://github.com/googleapis/librarian.git"},
1465-
})
1463+
}
14661464

14671465
status := make(git.Status)
14681466
status["file.txt"] = &git.FileStatus{Worktree: git.Modified}
14691467
return &MockRepository{
14701468
Dir: t.TempDir(),
14711469
AddAllStatus: status,
1472-
RemotesValue: []*git.Remote{remote},
1470+
RemotesValue: []*gitrepo.Remote{remote},
14731471
CommitError: errors.New("commit error"),
14741472
}
14751473
},
@@ -1484,17 +1482,17 @@ func TestCommitAndPush(t *testing.T) {
14841482
{
14851483
name: "Push error",
14861484
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1487-
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1485+
remote := &gitrepo.Remote{
14881486
Name: "origin",
14891487
URLs: []string{"https://github.com/googleapis/librarian.git"},
1490-
})
1488+
}
14911489

14921490
status := make(git.Status)
14931491
status["file.txt"] = &git.FileStatus{Worktree: git.Modified}
14941492
return &MockRepository{
14951493
Dir: t.TempDir(),
14961494
AddAllStatus: status,
1497-
RemotesValue: []*git.Remote{remote},
1495+
RemotesValue: []*gitrepo.Remote{remote},
14981496
PushError: errors.New("push error"),
14991497
}
15001498
},
@@ -1509,17 +1507,17 @@ func TestCommitAndPush(t *testing.T) {
15091507
{
15101508
name: "Create PR body error",
15111509
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1512-
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1510+
remote := &gitrepo.Remote{
15131511
Name: "origin",
15141512
URLs: []string{"https://github.com/googleapis/librarian.git"},
1515-
})
1513+
}
15161514

15171515
status := make(git.Status)
15181516
status["file.txt"] = &git.FileStatus{Worktree: git.Modified}
15191517
return &MockRepository{
15201518
Dir: t.TempDir(),
15211519
AddAllStatus: status,
1522-
RemotesValue: []*git.Remote{remote},
1520+
RemotesValue: []*gitrepo.Remote{remote},
15231521
}
15241522
},
15251523
setupMockClient: func(t *testing.T) GitHubClient {
@@ -1534,17 +1532,17 @@ func TestCommitAndPush(t *testing.T) {
15341532
{
15351533
name: "Create pull request error",
15361534
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1537-
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1535+
remote := &gitrepo.Remote{
15381536
Name: "origin",
15391537
URLs: []string{"https://github.com/googleapis/librarian.git"},
1540-
})
1538+
}
15411539

15421540
status := make(git.Status)
15431541
status["file.txt"] = &git.FileStatus{Worktree: git.Modified}
15441542
return &MockRepository{
15451543
Dir: t.TempDir(),
15461544
AddAllStatus: status,
1547-
RemotesValue: []*git.Remote{remote},
1545+
RemotesValue: []*gitrepo.Remote{remote},
15481546
}
15491547
},
15501548
setupMockClient: func(t *testing.T) GitHubClient {
@@ -1561,14 +1559,14 @@ func TestCommitAndPush(t *testing.T) {
15611559
{
15621560
name: "No changes to commit",
15631561
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1564-
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1562+
remote := &gitrepo.Remote{
15651563
Name: "origin",
15661564
URLs: []string{"https://github.com/googleapis/librarian.git"},
1567-
})
1565+
}
15681566
return &MockRepository{
15691567
Dir: t.TempDir(),
15701568
AddAllStatus: git.Status{}, // Clean status
1571-
RemotesValue: []*git.Remote{remote},
1569+
RemotesValue: []*gitrepo.Remote{remote},
15721570
}
15731571
},
15741572
setupMockClient: func(t *testing.T) GitHubClient {
@@ -1580,16 +1578,16 @@ func TestCommitAndPush(t *testing.T) {
15801578
{
15811579
name: "create_a_comment_on_generation_pr_error",
15821580
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1583-
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1581+
remote := &gitrepo.Remote{
15841582
Name: "origin",
15851583
URLs: []string{"https://github.com/googleapis/librarian.git"},
1586-
})
1584+
}
15871585
status := make(git.Status)
15881586
status["file.txt"] = &git.FileStatus{Worktree: git.Modified}
15891587
return &MockRepository{
15901588
Dir: t.TempDir(),
15911589
AddAllStatus: status,
1592-
RemotesValue: []*git.Remote{remote},
1590+
RemotesValue: []*gitrepo.Remote{remote},
15931591
}
15941592
},
15951593
setupMockClient: func(t *testing.T) GitHubClient {

‎internal/librarian/mocks_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ type MockRepository struct {
307307
AddAllStatus git.Status
308308
AddAllError error
309309
CommitError error
310-
RemotesValue []*git.Remote
310+
RemotesValue []*gitrepo.Remote
311311
RemotesError error
312312
CommitCalls int
313313
GetCommitError error
@@ -346,7 +346,7 @@ func (m *MockRepository) Commit(msg string) error {
346346
return m.CommitError
347347
}
348348

349-
func (m *MockRepository) Remotes() ([]*git.Remote, error) {
349+
func (m *MockRepository) Remotes() ([]*gitrepo.Remote, error) {
350350
if m.RemotesError != nil {
351351
return nil, m.RemotesError
352352
}

‎internal/librarian/release_init_test.go‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,13 @@ import (
2222
"strings"
2323
"testing"
2424

25-
gogitConfig "github.com/go-git/go-git/v5/config"
26-
"github.com/go-git/go-git/v5/plumbing"
27-
"github.com/go-git/go-git/v5/storage/memory"
28-
"gopkg.in/yaml.v3"
29-
30-
"github.com/googleapis/librarian/internal/conventionalcommits"
31-
3225
"github.com/go-git/go-git/v5"
33-
34-
"github.com/googleapis/librarian/internal/gitrepo"
35-
26+
"github.com/go-git/go-git/v5/plumbing"
3627
"github.com/google/go-cmp/cmp"
37-
3828
"github.com/googleapis/librarian/internal/config"
29+
"github.com/googleapis/librarian/internal/conventionalcommits"
30+
"github.com/googleapis/librarian/internal/gitrepo"
31+
"gopkg.in/yaml.v3"
3932
)
4033

4134
func TestNewInitRunner(t *testing.T) {
@@ -93,10 +86,12 @@ func TestInitRun(t *testing.T) {
9386
mockRepoWithReleasableUnit := &MockRepository{
9487
Dir: t.TempDir(),
9588
AddAllStatus: gitStatus,
96-
RemotesValue: []*git.Remote{git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
97-
Name: "origin",
98-
URLs: []string{"https://github.com/googleapis/librarian.git"},
99-
})},
89+
RemotesValue: []*gitrepo.Remote{
90+
{
91+
Name: "origin",
92+
URLs: []string{"https://github.com/googleapis/librarian.git"},
93+
},
94+
},
10095
ChangedFilesInCommitValue: []string{"dir1/file.txt"},
10196
GetCommitsForPathsSinceTagValue: []*gitrepo.Commit{
10297
{
@@ -667,10 +662,12 @@ func TestInitRun(t *testing.T) {
667662
},
668663
repo: &MockRepository{
669664
Dir: t.TempDir(),
670-
RemotesValue: []*git.Remote{git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
671-
Name: "origin",
672-
URLs: []string{"https://github.com/googleapis/librarian.git"},
673-
})},
665+
RemotesValue: []*gitrepo.Remote{
666+
{
667+
Name: "origin",
668+
URLs: []string{"https://github.com/googleapis/librarian.git"},
669+
},
670+
},
674671
ChangedFilesInCommitValue: []string{"file.txt"},
675672
GetCommitsForPathsSinceTagValue: []*gitrepo.Commit{
676673
{
@@ -888,10 +885,12 @@ func TestInitRun(t *testing.T) {
888885
repo: &MockRepository{
889886
Dir: t.TempDir(),
890887
AddAllStatus: gitStatus,
891-
RemotesValue: []*git.Remote{git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
892-
Name: "origin",
893-
URLs: []string{"https://github.com/googleapis/librarian.git"},
894-
})},
888+
RemotesValue: []*gitrepo.Remote{
889+
{
890+
Name: "origin",
891+
URLs: []string{"https://github.com/googleapis/librarian.git"},
892+
},
893+
},
895894
ChangedFilesInCommitValue: []string{"dir1/file.txt"},
896895
GetCommitsForPathsSinceTagValue: []*gitrepo.Commit{
897896
{

0 commit comments

Comments
 (0)