Skip to content

Commit d6fa957

Browse files
authored
fix(librarian): bump version while updating libraries for release (#1842)
Also refactored the function a little as it takes a pointer to data so no need to return the data -- mutate it instead. Fixes: #1830
1 parent 1321930 commit d6fa957

File tree

2 files changed

+21
-32
lines changed

2 files changed

+21
-32
lines changed

internal/librarian/release_init.go

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -119,17 +119,15 @@ func (r *initRunner) runInitCommand(ctx context.Context, outputDir string) error
119119
}
120120
src := r.repo.GetDir()
121121

122-
for i, library := range r.state.Libraries {
122+
for _, library := range r.state.Libraries {
123123
if r.cfg.Library != "" {
124124
if r.cfg.Library != library.ID {
125125
continue
126126
}
127127
// Only update one library with the given library ID.
128-
updatedLibrary, err := updateLibrary(r.repo, library, r.cfg.LibraryVersion)
129-
if err != nil {
128+
if err := updateLibrary(r.repo, library, r.cfg.LibraryVersion); err != nil {
130129
return err
131130
}
132-
r.state.Libraries[i] = updatedLibrary
133131
if err := copyLibrary(dst, src, library); err != nil {
134132
return err
135133
}
@@ -138,11 +136,9 @@ func (r *initRunner) runInitCommand(ctx context.Context, outputDir string) error
138136
}
139137

140138
// Update all libraries.
141-
updatedLibrary, err := updateLibrary(r.repo, library, r.cfg.LibraryVersion)
142-
if err != nil {
139+
if err := updateLibrary(r.repo, library, r.cfg.LibraryVersion); err != nil {
143140
return err
144141
}
145-
r.state.Libraries[i] = updatedLibrary
146142
if err := copyLibrary(dst, src, library); err != nil {
147143
return err
148144
}
@@ -199,25 +195,10 @@ func (r *initRunner) runInitCommand(ctx context.Context, outputDir string) error
199195
// 2. Override the library version if libraryVersion is not empty.
200196
//
201197
// 3. Set the library's release trigger to true.
202-
func updateLibrary(repo gitrepo.Repository, library *config.LibraryState, libraryVersion string) (*config.LibraryState, error) {
203-
updatedLibrary, err := getChangesOf(repo, library)
204-
if err != nil {
205-
return nil, fmt.Errorf("failed to update library, %s: %w", library.ID, err)
206-
}
207-
208-
if libraryVersion != "" {
209-
updatedLibrary.Version = libraryVersion
210-
}
211-
updatedLibrary.ReleaseTriggered = true
212-
213-
return updatedLibrary, nil
214-
}
215-
216-
// getChangesOf gets commit history of the given library.
217-
func getChangesOf(repo gitrepo.Repository, library *config.LibraryState) (*config.LibraryState, error) {
198+
func updateLibrary(repo gitrepo.Repository, library *config.LibraryState, libraryVersion string) error {
218199
commits, err := GetConventionalCommitsSinceLastRelease(repo, library)
219200
if err != nil {
220-
return nil, fmt.Errorf("failed to fetch conventional commits for library, %s: %w", library.ID, err)
201+
return fmt.Errorf("failed to fetch conventional commits for library, %s: %w", library.ID, err)
221202
}
222203

223204
changes := make([]*config.Change, 0)
@@ -236,10 +217,19 @@ func getChangesOf(repo gitrepo.Repository, library *config.LibraryState) (*confi
236217
CommitHash: commit.SHA,
237218
})
238219
}
220+
if len(changes) == 0 {
221+
// Don't update the library at all
222+
return nil
223+
}
239224

225+
nextVersion, err := NextVersion(commits, library.Version, libraryVersion)
226+
if err != nil {
227+
return err
228+
}
240229
library.Changes = changes
241-
242-
return library, nil
230+
library.Version = nextVersion
231+
library.ReleaseTriggered = true
232+
return nil
243233
}
244234

245235
// getChangeType gets the type of the commit, adding an escalation mark (!) if

internal/librarian/release_init_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ func TestInitRun(t *testing.T) {
294294
partialRepo: t.TempDir(),
295295
},
296296
wantErr: true,
297-
wantErrMsg: "failed to update library",
297+
wantErrMsg: "failed to fetch conventional commits for library",
298298
},
299299
{
300300
name: "failed to commit and push",
@@ -481,7 +481,7 @@ func TestUpdateLibrary(t *testing.T) {
481481
},
482482
want: &config.LibraryState{
483483
ID: "one-id",
484-
Version: "1.2.3",
484+
Version: "2.0.0",
485485
SourceRoots: []string{
486486
"one/path",
487487
"two/path",
@@ -519,12 +519,11 @@ func TestUpdateLibrary(t *testing.T) {
519519
} {
520520
t.Run(test.name, func(t *testing.T) {
521521
var err error
522-
var updated *config.LibraryState
523522
if test.repo != nil {
524-
updated, err = updateLibrary(test.repo, test.library, test.libraryVersion)
523+
err = updateLibrary(test.repo, test.library, test.libraryVersion)
525524
} else {
526525
repo := setupRepoForGetCommits(t, test.pathAndMessages, test.tags)
527-
updated, err = updateLibrary(repo, test.library, test.libraryVersion)
526+
err = updateLibrary(repo, test.library, test.libraryVersion)
528527
}
529528

530529
if test.wantErr {
@@ -542,7 +541,7 @@ func TestUpdateLibrary(t *testing.T) {
542541
if err != nil {
543542
t.Errorf("failed to run getChangesOf(): %q", err.Error())
544543
}
545-
if diff := cmp.Diff(test.want, updated, cmpopts.IgnoreFields(config.Change{}, "CommitHash")); diff != "" {
544+
if diff := cmp.Diff(test.want, test.library, cmpopts.IgnoreFields(config.Change{}, "CommitHash")); diff != "" {
546545
t.Errorf("state mismatch (-want +got):\n%s", diff)
547546
}
548547
})

0 commit comments

Comments
 (0)