From 590a834373d91725624c30adf0ef90853c95cb54 Mon Sep 17 00:00:00 2001 From: Ivan Palladino Date: Wed, 12 Jun 2019 22:27:22 +0200 Subject: [PATCH 1/2] cmd/gopherbot, maintner: add the author of the change to the notification on github When a new change is pushed, the bot posts a message on Github following this new format: Change https://golang.org/cl/NNNN by AUTHOR mentions this issue A new method to the CL's API in the maintner package is added to provide the related absolute url. Fixes #26816 --- cmd/gopherbot/gopherbot.go | 9 +++++---- maintner/gerrit.go | 10 ++++++++++ maintner/gerrit_test.go | 17 +++++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/cmd/gopherbot/gopherbot.go b/cmd/gopherbot/gopherbot.go index 45ef7349a2..89450a24a0 100644 --- a/cmd/gopherbot/gopherbot.go +++ b/cmd/gopherbot/gopherbot.go @@ -899,7 +899,7 @@ func (b *gopherbot) closeStaleWaitingForInfo(ctx context.Context) error { } -// cl2issue writes "Change https://golang.org/cl/NNNN mentions this issue" +// cl2issue writes "Change https://golang.org/cl/NNNN by AUTHOR mentions this issue" // and the change summary on GitHub when a new Gerrit change references a GitHub issue. func (b *gopherbot) cl2issue(ctx context.Context) error { monthAgo := time.Now().Add(-30 * 24 * time.Hour) @@ -915,6 +915,7 @@ func (b *gopherbot) cl2issue(ctx context.Context) error { // already processed this issue. return nil } + prefix := fmt.Sprintf("Change %s", cl.URL().String()) for _, ref := range cl.GitHubIssueRefs { if id := ref.Repo.ID(); id.Owner != "golang" || id.Repo != "go" { continue @@ -924,9 +925,8 @@ func (b *gopherbot) cl2issue(ctx context.Context) error { continue } hasComment := false - substr := fmt.Sprintf("%d mentions this issue", cl.Number) gi.ForeachComment(func(c *maintner.GitHubComment) error { - if strings.Contains(c.Body, substr) { + if strings.HasPrefix(c.Body, prefix) { hasComment = true return errStopIteration } @@ -934,7 +934,8 @@ func (b *gopherbot) cl2issue(ctx context.Context) error { }) if !hasComment { printIssue("cl2issue", gi) - msg := fmt.Sprintf("Change https://golang.org/cl/%d mentions this issue: `%s`", cl.Number, cl.Commit.Summary()) + author := cl.Owner().Name() + msg := fmt.Sprintf("%s by %s mentions this issue: `%s`", prefix, author, cl.Commit.Summary()) if err := b.addGitHubComment(ctx, "golang", "go", gi.Number, msg); err != nil { return err } diff --git a/maintner/gerrit.go b/maintner/gerrit.go index acf5f45a55..a0c358bbba 100644 --- a/maintner/gerrit.go +++ b/maintner/gerrit.go @@ -509,6 +509,16 @@ func (cl *GerritCL) CommitAtVersion(version int32) *GitCommit { return cl.Project.commit[hash] } +// URL returns the absolute url of the CL. +func (cl *GerritCL) URL() *url.URL { + s := fmt.Sprintf("https://golang.org/cl/%d", cl.Number) + u, err := url.Parse(s) + if err != nil { + return &url.URL{} + } + return u +} + func (cl *GerritCL) updateGithubIssueRefs() { gp := cl.Project gerrit := gp.gerrit diff --git a/maintner/gerrit_test.go b/maintner/gerrit_test.go index 4e973b3e83..d87d59582e 100644 --- a/maintner/gerrit_test.go +++ b/maintner/gerrit_test.go @@ -559,3 +559,20 @@ func TestGerritHashtagsLen(t *testing.T) { } } } + +func TestGerritURL(t *testing.T) { + tests := []struct { + set int32 + want string + }{ + {0, "https://golang.org/cl/0"}, + {1238812784, "https://golang.org/cl/1238812784"}, + } + for _, tt := range tests { + cl := &GerritCL{Number: tt.set} + got := cl.URL().String() + if got != tt.want { + t.Errorf("For set %d, url = %#v; want %v", tt.set, got, tt.want) + } + } +} From ebf4772f9e0ee1ba5d28f19c82ad50728975f4a8 Mon Sep 17 00:00:00 2001 From: Ivan Palladino Date: Thu, 22 Aug 2019 19:35:34 +0200 Subject: [PATCH 2/2] maintner: removed the cl url generator Restored the maintner file moving the cl url generator to the gopherbot logic as in the previous version. --- cmd/gopherbot/gopherbot.go | 2 +- maintner/gerrit.go | 10 ---------- maintner/gerrit_test.go | 17 ----------------- 3 files changed, 1 insertion(+), 28 deletions(-) diff --git a/cmd/gopherbot/gopherbot.go b/cmd/gopherbot/gopherbot.go index 89450a24a0..28bac4a22c 100644 --- a/cmd/gopherbot/gopherbot.go +++ b/cmd/gopherbot/gopherbot.go @@ -915,7 +915,7 @@ func (b *gopherbot) cl2issue(ctx context.Context) error { // already processed this issue. return nil } - prefix := fmt.Sprintf("Change %s", cl.URL().String()) + prefix := fmt.Sprintf("Change https://golang.org/cl/%d", cl.Number) for _, ref := range cl.GitHubIssueRefs { if id := ref.Repo.ID(); id.Owner != "golang" || id.Repo != "go" { continue diff --git a/maintner/gerrit.go b/maintner/gerrit.go index a0c358bbba..acf5f45a55 100644 --- a/maintner/gerrit.go +++ b/maintner/gerrit.go @@ -509,16 +509,6 @@ func (cl *GerritCL) CommitAtVersion(version int32) *GitCommit { return cl.Project.commit[hash] } -// URL returns the absolute url of the CL. -func (cl *GerritCL) URL() *url.URL { - s := fmt.Sprintf("https://golang.org/cl/%d", cl.Number) - u, err := url.Parse(s) - if err != nil { - return &url.URL{} - } - return u -} - func (cl *GerritCL) updateGithubIssueRefs() { gp := cl.Project gerrit := gp.gerrit diff --git a/maintner/gerrit_test.go b/maintner/gerrit_test.go index d87d59582e..4e973b3e83 100644 --- a/maintner/gerrit_test.go +++ b/maintner/gerrit_test.go @@ -559,20 +559,3 @@ func TestGerritHashtagsLen(t *testing.T) { } } } - -func TestGerritURL(t *testing.T) { - tests := []struct { - set int32 - want string - }{ - {0, "https://golang.org/cl/0"}, - {1238812784, "https://golang.org/cl/1238812784"}, - } - for _, tt := range tests { - cl := &GerritCL{Number: tt.set} - got := cl.URL().String() - if got != tt.want { - t.Errorf("For set %d, url = %#v; want %v", tt.set, got, tt.want) - } - } -}