diff options
| author | Dmitri Shuralyov <dmitshur@golang.org> | 2023-05-10 08:53:08 -0400 |
|---|---|---|
| committer | Gopher Robot <gobot@golang.org> | 2023-06-22 19:53:14 +0000 |
| commit | 1ee98ad07b3b077731a919d3cf37e7bfc1741fa3 (patch) | |
| tree | bced9d1f694e7adbb3a2f1a35415777af96ba134 /git-codereview | |
| parent | d285cae025f6e6848090ddb748aade98677328db (diff) | |
| download | go-x-review-1ee98ad07b3b077731a919d3cf37e7bfc1741fa3.tar.xz | |
git-codereview: fix mail lookup for dmitshur short name
CL 2111 introduced a simple algorithm for expanding short user names
that works locally and generally doesn't need any extra configuration
to do the right thing. It's been serving for almost a decade.
Unfortunately I'm running into an edge where I see no good option but
to add a special case. Gerrit inserts "Reviewed-by" footers both for
Code-Review +2 and +1 votes without any way to distinguish between them,
and my @google.com account unavoidably gets more hits since I use it for
the go.dev/s/needs-review process, whenever I leave a normal +2, and on
my own CLs.
The simplest fix seems to be to exclude it from short matches and let
the existing algorithm do the rest.
Change-Id: I92da69f1cd87b4c89816547015cffbed3eaaeaf6
Reviewed-on: https://go-review.googlesource.com/c/review/+/494175
Reviewed-by: MKaslann1 <mkaslann1@outlook.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Diffstat (limited to 'git-codereview')
| -rw-r--r-- | git-codereview/mail.go | 8 | ||||
| -rw-r--r-- | git-codereview/mail_test.go | 13 |
2 files changed, 20 insertions, 1 deletions
diff --git a/git-codereview/mail.go b/git-codereview/mail.go index c9bb567..5d385f9 100644 --- a/git-codereview/mail.go +++ b/git-codereview/mail.go @@ -254,13 +254,19 @@ func mailLookup(short string) string { short += "@" for _, r := range reviewers { - if strings.HasPrefix(r.addr, short) { + if strings.HasPrefix(r.addr, short) && !shortOptOut[r.addr] { return r.addr } } return "" } +// shortOptOut lists email addresses whose owners have opted out +// from consideration for purposes of expanding short user names. +var shortOptOut = map[string]bool{ + "dmitshur@google.com": true, // My @golang.org is primary; @google.com is used for +1 only. +} + // loadReviewers reads the reviewer list from the current git repo // and leaves it in the global variable reviewers. // See the comment on mailLookup for a description of how the diff --git a/git-codereview/mail_test.go b/git-codereview/mail_test.go index a376ea3..d3ef181 100644 --- a/git-codereview/mail_test.go +++ b/git-codereview/mail_test.go @@ -181,6 +181,10 @@ var reviewerLog = []string{ "Reviewer 1 <r1@golang.org>", "Other <other@golang.org>", "<anon@golang.org>", + "Reviewer 2 <r2@new.example>", + "Reviewer 2 <r2@old.example>", + "Reviewer 2 <r2@old.example>", + "Reviewer 2 <r2@old.example>", } func TestMailShort(t *testing.T) { @@ -226,6 +230,15 @@ func TestMailShort(t *testing.T) { testMainDied(t, "mail", "-r", "other", "-r", "anon,r1,missing") testPrintedStderr(t, "unknown reviewer: missing") + + // Test shortOptOut. + orig := shortOptOut + defer func() { shortOptOut = orig }() + shortOptOut = map[string]bool{"r2@old.example": true} + testMain(t, "mail", "-r", "r2") + testRan(t, + "git push -q origin HEAD:refs/for/main%r=r2@new.example", + "git tag --no-sign -f work.mailed "+h) } func TestWIP(t *testing.T) { |
