cmd/gopherbot: don't auto-assign author as reviewer
Filter reviewer list before assignment, by the email address of the
commit author.
Fixes golang/go#27533
Change-Id: I39061e3277bcad9b77b3ce7b5d4be69d9b351ac7
Reviewed-on: https://go-review.googlesource.com/c/140038
Reviewed-by: Andrew Bonventre <andybons@golang.org>
diff --git a/cmd/gopherbot/gopherbot.go b/cmd/gopherbot/gopherbot.go
index 43bc166..6698c7a 100644
--- a/cmd/gopherbot/gopherbot.go
+++ b/cmd/gopherbot/gopherbot.go
@@ -1675,7 +1675,8 @@
return nil
}
- merged := mergeOwnersEntries(entries)
+ authorEmail := cl.Commit.Author.Email()
+ merged := mergeOwnersEntries(entries, authorEmail)
if len(merged.Primary) == 0 && len(merged.Secondary) == 0 {
// No owners found for the change. Add the #no-owners tag.
log.Printf("Adding no-owners tag to change %s...", changeURL)
@@ -1840,8 +1841,10 @@
// primary and secondary users into a single entry.
// If a user is a primary in one entry but secondary on another, they are
// primary in the returned entry.
+// If a users email matches the authorEmail, the the user is omitted from the
+// result.
// The resulting order of the entries is non-deterministic.
-func mergeOwnersEntries(entries []*owners.Entry) *owners.Entry {
+func mergeOwnersEntries(entries []*owners.Entry, authorEmail string) *owners.Entry {
var result owners.Entry
pm := make(map[owners.Owner]bool)
for _, e := range entries {
@@ -1858,10 +1861,14 @@
}
}
for o := range pm {
- result.Primary = append(result.Primary, o)
+ if o.GerritEmail != authorEmail {
+ result.Primary = append(result.Primary, o)
+ }
}
for o := range sm {
- result.Secondary = append(result.Secondary, o)
+ if o.GerritEmail != authorEmail {
+ result.Secondary = append(result.Secondary, o)
+ }
}
return &result
}
diff --git a/cmd/gopherbot/gopherbot_test.go b/cmd/gopherbot/gopherbot_test.go
index 5786b8d..1616c78 100644
--- a/cmd/gopherbot/gopherbot_test.go
+++ b/cmd/gopherbot/gopherbot_test.go
@@ -204,7 +204,7 @@
"some labels already present in maintner",
&maintner.GitHubIssue{
Labels: map[int64]*maintner.GitHubLabel{
- 0: &maintner.GitHubLabel{Name: "NeedsDecision"},
+ 0: {Name: "NeedsDecision"},
},
},
[]string{"foo", "NeedsDecision"},
@@ -214,7 +214,7 @@
"all labels already present in maintner",
&maintner.GitHubIssue{
Labels: map[int64]*maintner.GitHubLabel{
- 0: &maintner.GitHubLabel{Name: "NeedsDecision"},
+ 0: {Name: "NeedsDecision"},
},
},
[]string{"NeedsDecision"},
@@ -265,7 +265,7 @@
"basic remove",
&maintner.GitHubIssue{
Labels: map[int64]*maintner.GitHubLabel{
- 0: &maintner.GitHubLabel{Name: "NeedsFix"},
+ 0: {Name: "NeedsFix"},
},
},
[]string{"NeedsFix"},
@@ -281,7 +281,7 @@
"label not present in GitHub",
&maintner.GitHubIssue{
Labels: map[int64]*maintner.GitHubLabel{
- 0: &maintner.GitHubLabel{Name: "foo"},
+ 0: {Name: "foo"},
},
},
[]string{"foo"},
@@ -356,13 +356,15 @@
filippo = owners.Owner{GitHubUsername: "filippo", GerritEmail: "filippo@golang.org"}
)
testCases := []struct {
- desc string
- entries []*owners.Entry
- result *owners.Entry
+ desc string
+ entries []*owners.Entry
+ authorEmail string
+ result *owners.Entry
}{
{
"no entries",
nil,
+ "",
&owners.Entry{},
},
{
@@ -371,6 +373,7 @@
{Primary: []owners.Owner{andybons}},
{Primary: []owners.Owner{bradfitz}},
},
+ "",
&owners.Entry{
Primary: []owners.Owner{andybons, bradfitz},
},
@@ -381,6 +384,7 @@
{Secondary: []owners.Owner{andybons}},
{Secondary: []owners.Owner{filippo}},
},
+ "",
&owners.Entry{
Secondary: []owners.Owner{andybons, filippo},
},
@@ -391,16 +395,37 @@
{Primary: []owners.Owner{andybons, filippo}},
{Secondary: []owners.Owner{filippo}},
},
+ "",
&owners.Entry{
Primary: []owners.Owner{andybons, filippo},
},
},
+ {
+ "primary filter",
+ []*owners.Entry{
+ {Primary: []owners.Owner{filippo, andybons}},
+ },
+ filippo.GerritEmail,
+ &owners.Entry{
+ Primary: []owners.Owner{andybons},
+ },
+ },
+ {
+ "secondary filter",
+ []*owners.Entry{
+ {Secondary: []owners.Owner{filippo, andybons}},
+ },
+ filippo.GerritEmail,
+ &owners.Entry{
+ Secondary: []owners.Owner{andybons},
+ },
+ },
}
cmpFn := func(a, b owners.Owner) bool {
return a.GitHubUsername < b.GitHubUsername
}
for _, tc := range testCases {
- got := mergeOwnersEntries(tc.entries)
+ got := mergeOwnersEntries(tc.entries, tc.authorEmail)
if diff := cmp.Diff(got, tc.result, cmpopts.SortSlices(cmpFn)); diff != "" {
t.Errorf("%s: final entry results differ: (-got, +want)\n%s", tc.desc, diff)
}
