Skip to content

Commit 60b7d38

Browse files
authored
fix: group commit msg if they have the same piper id and subject (#2496)
Example generation PR: https://github.com/JoeWang1127/google-cloud-python/pull/3 Example release PR: https://github.com/JoeWang1127/google-cloud-python/pull/4 Fixes #2470
1 parent 31a496d commit 60b7d38

File tree

4 files changed

+321
-15
lines changed

4 files changed

+321
-15
lines changed

internal/librarian/release_init.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"log/slog"
2121
"os"
2222
"path/filepath"
23+
"slices"
24+
"strings"
2325

2426
"github.com/googleapis/librarian/internal/conventionalcommits"
2527
"github.com/googleapis/librarian/internal/docker"
@@ -207,10 +209,19 @@ func (r *initRunner) processLibrary(library *config.LibraryState) error {
207209
return r.updateLibrary(library, commits)
208210
}
209211

210-
// filterCommitsByLibraryID keeps the conventional commits that match a libaryID.
212+
// filterCommitsByLibraryID keeps the conventional commits if the given libraryID appears in the Footer or matches
213+
// the libraryID in the commit.
211214
func filterCommitsByLibraryID(commits []*conventionalcommits.ConventionalCommit, libraryID string) []*conventionalcommits.ConventionalCommit {
212215
var filteredCommits []*conventionalcommits.ConventionalCommit
213216
for _, commit := range commits {
217+
if commit.Footers != nil {
218+
ids, ok := commit.Footers["Library-IDs"]
219+
libraryIDs := strings.Split(ids, ",")
220+
if ok && slices.Contains(libraryIDs, libraryID) {
221+
filteredCommits = append(filteredCommits, commit)
222+
continue
223+
}
224+
}
214225
if commit.LibraryID == libraryID {
215226
filteredCommits = append(filteredCommits, commit)
216227
}

internal/librarian/release_init_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,6 +1128,75 @@ func TestFilterCommitsByLibraryID(t *testing.T) {
11281128
},
11291129
},
11301130
},
1131+
{
1132+
name: "some_commits_have_library_id_in_footer",
1133+
commits: []*conventionalcommits.ConventionalCommit{
1134+
{
1135+
LibraryID: "library-one",
1136+
Type: "feat",
1137+
Footers: map[string]string{
1138+
"Library-IDs": "library-one,library-two",
1139+
},
1140+
},
1141+
{
1142+
LibraryID: "library-two",
1143+
Type: "chore",
1144+
Footers: map[string]string{
1145+
"Library-IDs": "library-one,library-two",
1146+
},
1147+
},
1148+
{
1149+
LibraryID: "library-three",
1150+
Type: "deps",
1151+
},
1152+
},
1153+
LibraryID: "library-one",
1154+
want: []*conventionalcommits.ConventionalCommit{
1155+
{
1156+
LibraryID: "library-one",
1157+
Type: "feat",
1158+
Footers: map[string]string{
1159+
"Library-IDs": "library-one,library-two",
1160+
},
1161+
},
1162+
{
1163+
LibraryID: "library-two",
1164+
Type: "chore",
1165+
Footers: map[string]string{
1166+
"Library-IDs": "library-one,library-two",
1167+
},
1168+
},
1169+
},
1170+
},
1171+
{
1172+
name: "some_commits_have_library_id_that_is_prefix_of_another",
1173+
commits: []*conventionalcommits.ConventionalCommit{
1174+
{
1175+
LibraryID: "library-one",
1176+
Type: "feat",
1177+
Footers: map[string]string{
1178+
"Library-IDs": "library-one,library-one_suffix",
1179+
},
1180+
},
1181+
{
1182+
LibraryID: "library-one-suffix",
1183+
Type: "chore",
1184+
Footers: map[string]string{
1185+
"Library-IDs": "library-one-suffix",
1186+
},
1187+
},
1188+
},
1189+
LibraryID: "library-one",
1190+
want: []*conventionalcommits.ConventionalCommit{
1191+
{
1192+
LibraryID: "library-one",
1193+
Type: "feat",
1194+
Footers: map[string]string{
1195+
"Library-IDs": "library-one,library-one_suffix",
1196+
},
1197+
},
1198+
},
1199+
},
11311200
{
11321201
name: "no_commits_match_libraryID",
11331202
commits: []*conventionalcommits.ConventionalCommit{

internal/librarian/release_notes.go

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,11 @@ Language Image: {{.ImageVersion}}
9393
}).Parse(`BEGIN_COMMIT_OVERRIDE
9494
{{ range .Commits }}
9595
BEGIN_NESTED_COMMIT
96-
{{.Type}}: [{{.LibraryID}}] {{.Subject}}
96+
{{.Type}}: {{.Subject}}
9797
{{.Body}}
9898
9999
PiperOrigin-RevId: {{index .Footers "PiperOrigin-RevId"}}
100-
100+
Library-IDs: {{index .Footers "Library-IDs"}}
101101
Source-link: [googleapis/googleapis@{{shortSHA .CommitHash}}](https://github.com/googleapis/googleapis/commit/{{shortSHA .CommitHash}})
102102
END_NESTED_COMMIT
103103
{{ end }}
@@ -182,20 +182,20 @@ func formatGenerationPRBody(repo gitrepo.Repository, state *config.LibrarianStat
182182
// because this function will return early if no conventional commit is found
183183
// since last generation.
184184
startSHA := startCommit.Hash.String()
185-
185+
groupedCommits := groupByIDAndSubject(allCommits)
186186
// Sort the slice by commit time in reverse order,
187187
// so that the latest commit appears first.
188-
sort.Slice(allCommits, func(i, j int) bool {
189-
return allCommits[i].When.After(allCommits[j].When)
188+
sort.Slice(groupedCommits, func(i, j int) bool {
189+
return groupedCommits[i].When.After(groupedCommits[j].When)
190190
})
191-
endSHA := allCommits[0].CommitHash
191+
endSHA := groupedCommits[0].CommitHash
192192
librarianVersion := cli.Version()
193193
data := &generationPRBody{
194194
StartSHA: startSHA,
195195
EndSHA: endSHA,
196196
LibrarianVersion: librarianVersion,
197197
ImageVersion: state.Image,
198-
Commits: allCommits,
198+
Commits: groupedCommits,
199199
FailedLibraries: failedLibraries,
200200
}
201201
var out bytes.Buffer
@@ -237,6 +237,44 @@ func findLatestGenerationCommit(repo gitrepo.Repository, state *config.Librarian
237237
return res, nil
238238
}
239239

240+
// groupByIDAndSubject aggregates conventional commits for ones have the same Piper ID and subject in the footer.
241+
func groupByIDAndSubject(commits []*conventionalcommits.ConventionalCommit) []*conventionalcommits.ConventionalCommit {
242+
var res []*conventionalcommits.ConventionalCommit
243+
idToCommits := make(map[string][]*conventionalcommits.ConventionalCommit)
244+
for _, commit := range commits {
245+
// a commit is not considering for grouping if it doesn't have a footer or
246+
// the footer doesn't have a Piper ID.
247+
if commit.Footers == nil {
248+
commit.Footers = make(map[string]string)
249+
commit.Footers["Library-IDs"] = commit.LibraryID
250+
res = append(res, commit)
251+
continue
252+
}
253+
254+
id, ok := commit.Footers["PiperOrigin-RevId"]
255+
if !ok {
256+
commit.Footers["Library-IDs"] = commit.LibraryID
257+
res = append(res, commit)
258+
continue
259+
}
260+
261+
key := fmt.Sprintf("%s-%s", id, commit.Subject)
262+
idToCommits[key] = append(idToCommits[key], commit)
263+
}
264+
265+
for _, groupCommits := range idToCommits {
266+
var ids []string
267+
for _, commit := range groupCommits {
268+
ids = append(ids, commit.LibraryID)
269+
}
270+
firstCommit := groupCommits[0]
271+
firstCommit.Footers["Library-IDs"] = strings.Join(ids, ",")
272+
res = append(res, firstCommit)
273+
}
274+
275+
return res
276+
}
277+
240278
// formatReleaseNotes generates the body for a release pull request.
241279
func formatReleaseNotes(state *config.LibrarianState, ghRepo *github.Repository) (string, error) {
242280
librarianVersion := cli.Version()

0 commit comments

Comments
 (0)