feat(internal/gapicgen): change commit formatting to match standard (#…

…3500)

Changing the commit formatting so that it can be parsed as a footer
with a conventional commits parser. Here is an example of what a
commit looked like previously:

  - docs(dialogflow): update comments on parameters and validation result.

  PiperOrigin-RevId: 348673154
  Source-Link: googleapis/googleapis@0795e3f

And what it will look like now:

  docs(dialogflow): update comments on parameters and validation result.
    PiperOrigin-RevId: 348673154
    Source-Link: googleapis/googleapis@0795e3f

internal/gapicgen/cmd/genbot/github.go

@@ -187,7 +187,7 @@ func (gc *GithubClient) CreateGenprotoPR(ctx context.Context, genprotoDir string
 	sb.WriteString(genprotoCommitBody)
 	if !hasCorrespondingPR {
 		sb.WriteString("\n\nThere is no corresponding google-cloud-go PR.\n")
-		sb.WriteString(formatChanges(changes, false))
+		sb.WriteString(generator.FormatChanges(changes, false))
 	}
 	body := sb.String()
 
@@ -264,7 +264,7 @@ func (gc *GithubClient) CreateGocloudPR(ctx context.Context, gocloudDir string,
 	} else {
 		sb.WriteString("\n\nThere is no corresponding genproto PR.\n")
 	}
-	sb.WriteString(formatChanges(changes, true))
+	sb.WriteString(generator.FormatChanges(changes, true))
 	body := sb.String()
 
 	c := exec.Command("/bin/bash", "-c", `
@@ -318,7 +318,7 @@ func (gc *GithubClient) AmendGenprotoPR(ctx context.Context, genprotoPRNum int,
 	var body strings.Builder
 	body.WriteString(genprotoCommitBody)
 	body.WriteString(fmt.Sprintf("\n\nCorresponding google-cloud-go PR: googleapis/google-cloud-go#%d\n", gocloudPRNum))
-	body.WriteString(formatChanges(changes, false))
+	body.WriteString(generator.FormatChanges(changes, false))
 	sBody := body.String()
 	c := exec.Command("/bin/bash", "-c", `
 set -ex
@@ -367,31 +367,3 @@ func (gc *GithubClient) MarkPRReadyForReview(ctx context.Context, repo string, n
 	}
 	return nil
 }
-
-func formatChanges(changes []*generator.ChangeInfo, onlyGapicChanges bool) string {
-	if len(changes) == 0 {
-		return ""
-	}
-	var sb strings.Builder
-	sb.WriteString("\nChanges:\n")
-	for _, c := range changes {
-		if onlyGapicChanges && !c.HasGapicChanges {
-			continue
-		}
-		sb.WriteString("- ")
-		ss := strings.Split(c.Body, "\n")
-		for i, s := range ss {
-			if i == 0 {
-				sb.WriteString(fmt.Sprintf("%s\n", s))
-				continue
-			}
-			if s == "" {
-				sb.WriteString("\n")
-				continue
-			}
-			sb.WriteString(fmt.Sprintf("  %s\n", s))
-		}
-		sb.WriteString("\n")
-	}
-	return sb.String()
-}

internal/gapicgen/cmd/genlocal/main.go

@@ -22,7 +22,6 @@ package main
 import (
 	"context"
 	"flag"
-	"fmt"
 	"io/ioutil"
 	"log"
 	"os"
@@ -93,12 +92,7 @@ func main() {
 	log.Println(gocloudDir)
 
 	if *verbose {
-		log.Println("Changes:")
-		fmt.Println()
-		for _, v := range changes {
-			fmt.Println("********************************************")
-			fmt.Println(v.Body)
-		}
+		log.Println(generator.FormatChanges(changes, false))
 	}
 }
 

internal/gapicgen/generator/git.go

@@ -22,9 +22,55 @@ import (
 
 // ChangeInfo represents a change and its associated metadata.
 type ChangeInfo struct {
-	Body            string
-	GoogleapisHash  string
-	HasGapicChanges bool
+	Body           string
+	Title          string
+	Package        string
+	GoogleapisHash string
+}
+
+// FormatChanges turns a slice of changes into formatted string that will match
+// the conventional commit footer pattern. This will allow these changes to be
+// parsed into the changelog.
+func FormatChanges(changes []*ChangeInfo, onlyGapicChanges bool) string {
+	if len(changes) == 0 {
+		return ""
+	}
+	var sb strings.Builder
+	sb.WriteString("\nChanges:\n\n")
+	for _, c := range changes {
+		if onlyGapicChanges && c.Package == "" {
+			continue
+		}
+
+		title := c.Title
+		if c.Package != "" {
+			// Try to add in pkg affected into conventional commit scope.
+			titleParts := strings.SplitN(c.Title, ":", 2)
+			if len(titleParts) == 2 {
+				// If a scope is already provided, remove it.
+				if i := strings.Index(titleParts[0], "("); i > 0 {
+					titleParts[0] = titleParts[0][:i]
+				}
+				titleParts[0] = fmt.Sprintf("%s(%s)", titleParts[0], c.Package)
+			}
+			title = strings.Join(titleParts, ":")
+		}
+		sb.WriteString(fmt.Sprintf("%s\n", title))
+
+		// Format the commit body to conventional commit footer standards.
+		splitBody := strings.Split(c.Body, "\n")
+		for i := range splitBody {
+			splitBody[i] = fmt.Sprintf("  %s", splitBody[i])
+		}
+		body := strings.Join(splitBody, "\n")
+		sb.WriteString(fmt.Sprintf("%s\n\n", body))
+	}
+	// If the buffer is empty except for the "Changes:" text return an empty
+	// string.
+	if sb.Len() == 11 {
+		return ""
+	}
+	return sb.String()
 }
 
 // ParseChangeInfo gets the ChangeInfo for a given googleapis hash.
@@ -40,15 +86,21 @@ func ParseChangeInfo(googleapisDir string, hashes []string) ([]*ChangeInfo, erro
 	for _, hash := range hashes {
 		// Get commit title and body
 		rawBody := bytes.NewBuffer(nil)
-		c := command("git", "show", "--pretty=format:%B", "-s", hash)
+		c := command("git", "show", "--pretty=format:%s~~%b", "-s", hash)
 		c.Stdout = rawBody
 		c.Dir = googleapisDir
 		if err := c.Run(); err != nil {
 			return nil, err
 		}
 
+		ss := strings.Split(rawBody.String(), "~~")
+		if len(ss) != 2 {
+			return nil, fmt.Errorf("expected two segments for commit, got %d: %q", len(ss), rawBody.String())
+		}
+		title, body := strings.TrimSpace(ss[0]), strings.TrimSpace(ss[1])
+
 		// Add link so corresponding googleapis commit.
-		body := fmt.Sprintf("%s\nSource-Link: https://github.com/googleapis/googleapis/commit/%s", strings.TrimSpace(rawBody.String()), hash)
+		body = fmt.Sprintf("%s\nSource-Link: https://github.com/googleapis/googleapis/commit/%s", body, hash)
 
 		// Try to map files updated to a package in google-cloud-go. Assumes only
 		// one servies protos are updated per commit. Multile versions are okay.
@@ -70,33 +122,12 @@ func ParseChangeInfo(googleapisDir string, hashes []string) ([]*ChangeInfo, erro
 				break
 			}
 		}
-		if pkg == "" {
-			changes = append(changes, &ChangeInfo{
-				Body:           body,
-				GoogleapisHash: hash,
-			})
-			continue
-		}
-
-		// Try to add in pkg affected into conventional commit scope.
-		bodyParts := strings.SplitN(body, "\n", 2)
-		if len(bodyParts) > 0 {
-			titleParts := strings.SplitN(bodyParts[0], ":", 2)
-			if len(titleParts) == 2 {
-				// If a scope is already provided, remove it.
-				if i := strings.Index(titleParts[0], "("); i > 0 {
-					titleParts[0] = titleParts[0][:i]
-				}
-				titleParts[0] = fmt.Sprintf("%s(%s)", titleParts[0], pkg)
-				bodyParts[0] = strings.Join(titleParts, ":")
-			}
-			body = strings.Join(bodyParts, "\n")
-		}
 
 		changes = append(changes, &ChangeInfo{
-			Body:            body,
-			GoogleapisHash:  hash,
-			HasGapicChanges: true,
+			Title:          title,
+			Body:           body,
+			Package:        pkg,
+			GoogleapisHash: hash,
 		})
 	}
 	return changes, nil

internal/gapicgen/generator/git_test.go

@@ -34,3 +34,63 @@ func TestParseConventionalCommitPkg(t *testing.T) {
 		})
 	}
 }
+
+func TestFormatChanges(t *testing.T) {
+	tests := []struct {
+		name       string
+		changes    []*ChangeInfo
+		onlyGapics bool
+		want       string
+	}{
+		{
+			name:    "basic",
+			changes: []*ChangeInfo{{Title: "fix: foo", Body: "bar"}},
+			want:    "\nChanges:\n\nfix: foo\n  bar\n\n",
+		},
+		{
+			name:    "breaking change",
+			changes: []*ChangeInfo{{Title: "feat!: breaking change", Body: "BREAKING CHANGE: The world is breaking."}},
+			want:    "\nChanges:\n\nfeat!: breaking change\n  BREAKING CHANGE: The world is breaking.\n\n",
+		},
+		{
+			name:    "multi-lined body indented",
+			changes: []*ChangeInfo{{Title: "fix: foo", Body: "bar\nbaz"}},
+			want:    "\nChanges:\n\nfix: foo\n  bar\n  baz\n\n",
+		},
+		{
+			name:    "multi-lined body indented, multiple changes",
+			changes: []*ChangeInfo{{Title: "fix: foo", Body: "bar\nbaz"}, {Title: "fix: baz", Body: "foo\nbar"}},
+			want:    "\nChanges:\n\nfix: foo\n  bar\n  baz\n\nfix: baz\n  foo\n  bar\n\n",
+		},
+		{
+			name:       "no package, filtered",
+			changes:    []*ChangeInfo{{Title: "fix: foo", Body: "bar"}},
+			onlyGapics: true,
+			want:       "",
+		},
+		{
+			name:    "with package",
+			changes: []*ChangeInfo{{Title: "fix: foo", Body: "bar", Package: "baz"}},
+			want:    "\nChanges:\n\nfix(baz): foo\n  bar\n\n",
+		},
+		{
+			name:    "multiple changes",
+			changes: []*ChangeInfo{{Title: "fix: foo", Body: "bar", Package: "foo"}, {Title: "fix: baz", Body: "bar"}},
+			want:    "\nChanges:\n\nfix(foo): foo\n  bar\n\nfix: baz\n  bar\n\n",
+		},
+		{
+			name:       "multiple changes, some filtered",
+			changes:    []*ChangeInfo{{Title: "fix: foo", Body: "bar", Package: "foo"}, {Title: "fix: baz", Body: "bar"}},
+			onlyGapics: true,
+			want:       "\nChanges:\n\nfix(foo): foo\n  bar\n\n",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := FormatChanges(tc.changes, tc.onlyGapics); got != tc.want {
+				t.Errorf("FormatChanges() = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}