Skip to content

Commit 215e5bd

Browse files
codyosscodyoss
authored
refactor(librarian): cleanup dead code (#2553)
I am going to be making some changes to how we process conventional commits to avoid re-processing information in the future. Updates: #2543
1 parent 12fef2b commit 215e5bd

File tree

4 files changed

+10
-56
lines changed

4 files changed

+10
-56
lines changed

‎internal/conventionalcommits/conventional_commits.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,18 +63,13 @@ type ConventionalCommit struct {
6363
Body string `yaml:"body" json:"body"`
6464
// LibraryID is the library ID the commit associated with.
6565
LibraryID string `yaml:"-" json:"-"`
66-
// Scope is the scope of the change.
67-
Scope string `yaml:"-" json:"-"`
6866
// Footers contain metadata (e.g,"BREAKING CHANGE", "Reviewed-by").
6967
// Repeated footer keys not supported, only first value is kept
7068
Footers map[string]string `yaml:"-" json:"-"`
7169
// IsBreaking indicates if the commit introduces a breaking change.
7270
IsBreaking bool `yaml:"-" json:"-"`
7371
// IsNested indicates if the commit is a nested commit.
7472
IsNested bool `yaml:"-" json:"-"`
75-
// SHA is the full commit hash.
76-
// Deprecated: use CommitHash instead.
77-
SHA string `yaml:"-" json:"source_commit_hash,omitempty"`
7873
// CommitHash is the full commit hash.
7974
CommitHash string `yaml:"-" json:"commit_hash,omitempty"`
8075
// When is the timestamp of the commit.
@@ -259,13 +254,11 @@ func parseSimpleCommit(commitPart commitPart, commit *gitrepo.Commit, libraryID
259254

260255
commits = append(commits, &ConventionalCommit{
261256
Type: header.Type,
262-
Scope: header.Scope,
263257
Subject: header.Description,
264258
LibraryID: libraryID,
265259
Footers: footers,
266260
IsBreaking: header.IsBreaking || footerIsBreaking,
267261
IsNested: commitPart.isNested,
268-
SHA: commit.Hash.String(),
269262
CommitHash: commit.Hash.String(),
270263
When: commit.When,
271264
})

‎internal/conventionalcommits/conventional_commits_test.go‎

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ func TestParseCommits(t *testing.T) {
4444
LibraryID: "example-id",
4545
IsNested: false,
4646
Footers: make(map[string]string),
47-
SHA: sha.String(),
4847
CommitHash: sha.String(),
4948
When: now,
5049
},
@@ -56,12 +55,10 @@ func TestParseCommits(t *testing.T) {
5655
want: []*ConventionalCommit{
5756
{
5857
Type: "feat",
59-
Scope: "scope",
6058
Subject: "add new feature",
6159
LibraryID: "example-id",
6260
IsNested: false,
6361
Footers: make(map[string]string),
64-
SHA: sha.String(),
6562
CommitHash: sha.String(),
6663
When: now,
6764
},
@@ -78,7 +75,6 @@ func TestParseCommits(t *testing.T) {
7875
IsBreaking: true,
7976
IsNested: false,
8077
Footers: make(map[string]string),
81-
SHA: sha.String(),
8278
CommitHash: sha.String(),
8379
When: now,
8480
},
@@ -94,7 +90,6 @@ func TestParseCommits(t *testing.T) {
9490
LibraryID: "example-id",
9591
IsNested: false,
9692
Footers: map[string]string{"Co-authored-by": "John Doe <[email protected]>"},
97-
SHA: sha.String(),
9893
CommitHash: sha.String(),
9994
When: now,
10095
},
@@ -113,7 +108,6 @@ func TestParseCommits(t *testing.T) {
113108
"Co-authored-by": "John Doe <[email protected]>",
114109
"Reviewed-by": "Jane Smith <[email protected]>",
115110
},
116-
SHA: sha.String(),
117111
CommitHash: sha.String(),
118112
When: now,
119113
},
@@ -134,7 +128,6 @@ func TestParseCommits(t *testing.T) {
134128
"PiperOrigin-RevId": "piper_cl_number",
135129
"Source-Link": "abcdefg1234567",
136130
},
137-
SHA: sha.String(),
138131
CommitHash: sha.String(),
139132
When: now,
140133
},
@@ -152,7 +145,6 @@ func TestParseCommits(t *testing.T) {
152145
IsNested: false,
153146
IsBreaking: true,
154147
Footers: map[string]string{"BREAKING CHANGE": "this is a breaking change"},
155-
SHA: sha.String(),
156148
CommitHash: sha.String(),
157149
When: now,
158150
},
@@ -170,7 +162,6 @@ func TestParseCommits(t *testing.T) {
170162
IsNested: false,
171163
IsBreaking: false,
172164
Footers: map[string]string{},
173-
SHA: sha.String(),
174165
CommitHash: sha.String(),
175166
When: now,
176167
},
@@ -187,7 +178,6 @@ func TestParseCommits(t *testing.T) {
187178
LibraryID: "example-id",
188179
IsNested: false,
189180
Footers: map[string]string{"Co-authored-by": "John Doe <[email protected]>"},
190-
SHA: sha.String(),
191181
CommitHash: sha.String(),
192182
When: now,
193183
},
@@ -205,7 +195,6 @@ func TestParseCommits(t *testing.T) {
205195
IsNested: false,
206196
IsBreaking: true,
207197
Footers: map[string]string{"BREAKING CHANGE": "this is a breaking change\nthat spans multiple lines."},
208-
SHA: sha.String(),
209198
CommitHash: sha.String(),
210199
When: now,
211200
},
@@ -225,13 +214,11 @@ END_COMMIT_OVERRIDE`,
225214
want: []*ConventionalCommit{
226215
{
227216
Type: "fix",
228-
Scope: "override",
229217
Subject: "this is the override message",
230218
Body: "This is the body of the override.",
231219
LibraryID: "example-id",
232220
IsNested: false,
233221
Footers: map[string]string{"Reviewed-by": "Jane Doe"},
234-
SHA: sha.String(),
235222
CommitHash: sha.String(),
236223
When: now,
237224
},
@@ -267,37 +254,31 @@ END_NESTED_COMMIT
267254
want: []*ConventionalCommit{
268255
{
269256
Type: "feat",
270-
Scope: "parser",
271257
Subject: "main feature",
272258
Body: "main commit body",
273259
LibraryID: "example-id",
274260
IsNested: false,
275261
Footers: map[string]string{},
276-
SHA: sha.String(),
277262
CommitHash: sha.String(),
278263
When: now,
279264
},
280265
{
281266
Type: "fix",
282-
Scope: "sub",
283267
Subject: "fix a bug",
284268
Body: "some details for the fix",
285269
LibraryID: "example-id",
286270
IsNested: true,
287271
Footers: map[string]string{},
288-
SHA: sha.String(),
289272
CommitHash: sha.String(),
290273
When: now,
291274
},
292275
{
293276
Type: "chore",
294-
Scope: "deps",
295277
Subject: "update deps",
296278
Body: "",
297279
LibraryID: "example-id",
298280
IsNested: true,
299281
Footers: map[string]string{},
300-
SHA: sha.String(),
301282
CommitHash: sha.String(),
302283
When: now,
303284
},
@@ -314,12 +295,10 @@ END_NESTED_COMMIT
314295
want: []*ConventionalCommit{
315296
{
316297
Type: "feat",
317-
Scope: "parser",
318298
Subject: "main feature 2nd line of title",
319299
LibraryID: "example-id",
320300
IsNested: false,
321301
Footers: map[string]string{},
322-
SHA: sha.String(),
323302
CommitHash: sha.String(),
324303
When: now,
325304
},
@@ -366,7 +345,6 @@ END_COMMIT_OVERRIDE
366345
LibraryID: "abc",
367346
IsNested: true,
368347
Footers: map[string]string{"PiperOrigin-RevId": "123456", "Source-Link": "fake-link"},
369-
SHA: sha.String(), // For each nested commit, the SHA should be the same (points to language repo's commit hash)
370348
CommitHash: sha.String(),
371349
When: now,
372350
},
@@ -377,7 +355,6 @@ END_COMMIT_OVERRIDE
377355
Body: "body of nested commit 2\n...",
378356
LibraryID: "abc",
379357
Footers: map[string]string{"PiperOrigin-RevId": "654321", "Source-Link": "fake-link"},
380-
SHA: sha.String(), // For each nested commit, the SHA should be the same (points to language repo's commit hash)
381358
CommitHash: sha.String(),
382359
When: now,
383360
},
@@ -400,13 +377,11 @@ END_NESTED_COMMIT`,
400377
want: []*ConventionalCommit{
401378
{
402379
Type: "fix",
403-
Scope: "override",
404380
Subject: "this is the override message",
405381
Body: "This is the body of the override.",
406382
LibraryID: "example-id",
407383
IsNested: false,
408384
Footers: map[string]string{"Reviewed-by": "Jane Doe"},
409-
SHA: sha.String(),
410385
CommitHash: sha.String(),
411386
When: now,
412387
},
@@ -444,7 +419,6 @@ END_COMMIT_OVERRIDE`,
444419
"PiperOrigin-RevId": "799242210",
445420
"Source-Link": "b738e78ed63effb7d199ed2d61c9e03291b6077f",
446421
},
447-
SHA: sha.String(),
448422
CommitHash: sha.String(),
449423
When: now,
450424
},
@@ -457,7 +431,6 @@ END_COMMIT_OVERRIDE`,
457431
"PiperOrigin-RevId": "799242210",
458432
"Source-Link": "b738e78ed63effb7d199ed2d61c9e03291b6077f",
459433
},
460-
SHA: sha.String(),
461434
CommitHash: sha.String(),
462435
When: now,
463436
},
@@ -470,7 +443,6 @@ END_COMMIT_OVERRIDE`,
470443
"PiperOrigin-RevId": "799242210",
471444
"Source-Link": "b738e78ed63effb7d199ed2d61c9e03291b6077f",
472445
},
473-
SHA: sha.String(),
474446
CommitHash: sha.String(),
475447
When: now,
476448
},
@@ -504,7 +476,6 @@ END_COMMIT_OVERRIDE`,
504476
LibraryID: "texttospeech",
505477
IsNested: true,
506478
Footers: map[string]string{},
507-
SHA: sha.String(),
508479
CommitHash: sha.String(),
509480
When: now,
510481
},
@@ -514,7 +485,6 @@ END_COMMIT_OVERRIDE`,
514485
LibraryID: "texttospeech",
515486
IsNested: true,
516487
Footers: map[string]string{},
517-
SHA: sha.String(),
518488
CommitHash: sha.String(),
519489
When: now,
520490
},
@@ -524,7 +494,6 @@ END_COMMIT_OVERRIDE`,
524494
LibraryID: "texttospeech",
525495
IsNested: true,
526496
Footers: map[string]string{},
527-
SHA: sha.String(),
528497
CommitHash: sha.String(),
529498
When: now,
530499
},
@@ -561,7 +530,6 @@ END_COMMIT_OVERRIDE`,
561530
LibraryID: "google-cloud-video-live-stream",
562531
IsNested: true,
563532
Footers: map[string]string{},
564-
SHA: sha.String(),
565533
CommitHash: sha.String(),
566534
When: now,
567535
},
@@ -571,7 +539,6 @@ END_COMMIT_OVERRIDE`,
571539
LibraryID: "google-cloud-eventarc",
572540
IsNested: true,
573541
Footers: map[string]string{},
574-
SHA: sha.String(),
575542
CommitHash: sha.String(),
576543
When: now,
577544
},
@@ -681,14 +648,13 @@ func TestConventionalCommit_MarshalJSON(t *testing.T) {
681648
Footers: map[string]string{
682649
"PiperOrigin-RevId": "12345",
683650
},
684-
SHA: "1234",
685651
CommitHash: "1234",
686652
}
687653
b, err := c.MarshalJSON()
688654
if err != nil {
689655
t.Fatalf("MarshalJSON() failed: %v", err)
690656
}
691-
want := `{"type":"feat","subject":"new feature","body":"body of feature","source_commit_hash":"1234","commit_hash":"1234","piper_cl_number":"12345"}`
657+
want := `{"type":"feat","subject":"new feature","body":"body of feature","commit_hash":"1234","piper_cl_number":"12345"}`
692658
if diff := cmp.Diff(want, string(b)); diff != "" {
693659
t.Errorf("MarshalJSON() mismatch (-want +got):\n%s", diff)
694660
}

‎internal/librarian/commit_version_analyzer_test.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -370,14 +370,12 @@ END_COMMIT_OVERRIDE`,
370370
},
371371
{
372372
Type: "feat",
373-
Scope: "foo",
374373
Subject: "another feature for foo",
375374
LibraryID: "foo",
376375
Footers: make(map[string]string),
377376
},
378377
{
379378
Type: "fix",
380-
Scope: "foo",
381379
Subject: "a fix for foo",
382380
LibraryID: "foo",
383381
Footers: make(map[string]string),
@@ -460,7 +458,7 @@ END_COMMIT_OVERRIDE`,
460458
if err != nil {
461459
t.Fatalf("getConventionalCommitsSinceLastRelease() failed: %v", err)
462460
}
463-
if diff := cmp.Diff(test.want, got, cmpopts.IgnoreFields(conventionalcommits.ConventionalCommit{}, "SHA", "CommitHash", "Body", "IsBreaking", "When")); diff != "" {
461+
if diff := cmp.Diff(test.want, got, cmpopts.IgnoreFields(conventionalcommits.ConventionalCommit{}, "CommitHash", "Body", "IsBreaking", "When")); diff != "" {
464462
t.Errorf("getConventionalCommitsSinceLastRelease() mismatch (-want +got):\n%s", diff)
465463
}
466464
})
@@ -505,7 +503,6 @@ func TestGetConventionalCommitsSinceLastGeneration(t *testing.T) {
505503
want: []*conventionalcommits.ConventionalCommit{
506504
{
507505
Type: "feat",
508-
Scope: "foo",
509506
Subject: "a feature",
510507
LibraryID: "foo",
511508
Footers: map[string]string{},
@@ -538,7 +535,6 @@ func TestGetConventionalCommitsSinceLastGeneration(t *testing.T) {
538535
want: []*conventionalcommits.ConventionalCommit{
539536
{
540537
Type: "feat",
541-
Scope: "foo",
542538
Subject: "a feature",
543539
LibraryID: "foo",
544540
Footers: map[string]string{},
@@ -611,7 +607,7 @@ func TestGetConventionalCommitsSinceLastGeneration(t *testing.T) {
611607
if err != nil {
612608
t.Fatalf("getConventionalCommitsSinceLastRelease() failed: %v", err)
613609
}
614-
if diff := cmp.Diff(test.want, got, cmpopts.IgnoreFields(conventionalcommits.ConventionalCommit{}, "SHA", "CommitHash", "Body", "IsBreaking", "When")); diff != "" {
610+
if diff := cmp.Diff(test.want, got, cmpopts.IgnoreFields(conventionalcommits.ConventionalCommit{}, "CommitHash", "Body", "IsBreaking", "When")); diff != "" {
615611
t.Errorf("getConventionalCommitsSinceLastRelease() mismatch (-want +got):\n%s", diff)
616612
}
617613
})

‎testdata/e2e_func.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,8 @@ func validateReleaseInitRequestJSON(state *librarianState) error {
287287
if change.PiperCLNumber == "" {
288288
return fmt.Errorf("validation error: library %s, change %d missing 'piper_cl_number'", lib.ID, j)
289289
}
290-
if change.SourceCommitHash == "" && change.CommitHash == "" {
291-
return fmt.Errorf("validation error: library %s, change %d missing 'source_commit_hash' or 'commit_hash'", lib.ID, j)
290+
if change.CommitHash == "" {
291+
return fmt.Errorf("validation error: library %s, change %d missing 'commit_hash'", lib.ID, j)
292292
}
293293

294294
// Validation logic to check for incorrect commit association.
@@ -531,10 +531,9 @@ type api struct {
531531
}
532532

533533
type change struct {
534-
Type string `json:"type" yaml:"type"`
535-
Subject string `json:"subject" yaml:"subject"`
536-
Body string `json:"body" yaml:"body"`
537-
PiperCLNumber string `json:"piper_cl_number" yaml:"piper_cl_number"`
538-
SourceCommitHash string `json:"source_commit_hash" yaml:"source_commit_hash"`
539-
CommitHash string `json:"commit_hash" yaml:"commit_hash"`
534+
Type string `json:"type" yaml:"type"`
535+
Subject string `json:"subject" yaml:"subject"`
536+
Body string `json:"body" yaml:"body"`
537+
PiperCLNumber string `json:"piper_cl_number" yaml:"piper_cl_number"`
538+
CommitHash string `json:"commit_hash" yaml:"commit_hash"`
540539
}

0 commit comments

Comments
 (0)