From d1e3d46c47c425581e2b149c07f8e27ffc373c7e Mon Sep 17 00:00:00 2001 From: Cody Oss <6331106+codyoss@users.noreply.github.com> Date: Mon, 11 Jan 2021 16:09:54 -0700 Subject: [PATCH] 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 | 34 +-------- internal/gapicgen/cmd/genlocal/main.go | 8 +-- internal/gapicgen/generator/git.go | 91 +++++++++++++++++-------- internal/gapicgen/generator/git_test.go | 60 ++++++++++++++++ 4 files changed, 125 insertions(+), 68 deletions(-) diff --git a/internal/gapicgen/cmd/genbot/github.go b/internal/gapicgen/cmd/genbot/github.go index ffef145f85f..bf84c722b10 100644 --- a/internal/gapicgen/cmd/genbot/github.go +++ b/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() -} diff --git a/internal/gapicgen/cmd/genlocal/main.go b/internal/gapicgen/cmd/genlocal/main.go index d61aec7b448..0f004a0e815 100644 --- a/internal/gapicgen/cmd/genlocal/main.go +++ b/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)) } } diff --git a/internal/gapicgen/generator/git.go b/internal/gapicgen/generator/git.go index 189bad5d816..42e7dfdbc76 100644 --- a/internal/gapicgen/generator/git.go +++ b/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 diff --git a/internal/gapicgen/generator/git_test.go b/internal/gapicgen/generator/git_test.go index b971259a4be..9d5f57e1fcc 100644 --- a/internal/gapicgen/generator/git_test.go +++ b/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) + } + }) + } +}