x/build/internal/gophers: first email must be Gerrit email #27517
Comments
|
Am I understanding this bug correctly? p := gophers.GetPerson("@FiloSottile")
got := p.Gerrit // "6195@62eb7196-b449-3ce5-99f1-c037f21e1705"
want := "filippo@golang.org"@andybons Do you know how many people, other than Filippo, are affected? Is it a part of resolving this issue to find that out? I think it can be checked in bulk by iterating over all the people in |
|
yes that's correct. as for others affected, that's part of this bug, yes. |
@andybons Could you please provide more information about that. I'd like to get a better understanding of the complete high-level picture here, so I can find the most effective way to resolve the underlying issue. Answering the "What did you do? / What did you expect to see? / What did you see instead?" questions (from the perspective of I want to be able to test my changes, and I might want to use Gerrit in the same way to verify whether my fix is complete. |
|
A Gerrit user has an email associated with their account (https://go-review.googlesource.com/settings/#Profile). When assigning a reviewer, the email used must match this (or another email that the user has entered in settings). When I say Gerrit user ID emails, I'm referring to those that look like To start, we should at least be treating the Gerrit user ID "emails" as definitely not elligible when specifying what someone's Gerrit email is (the one others should be using to assign a review to someone). You could theoretically use https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html to check whether an ID is a valid one to determine which email to associate with an individual as well. |
|
Change https://golang.org/cl/135456 mentions this issue: |
|
Change https://golang.org/cl/195062 mentions this issue: |
This is a cherry-pick of CL 135456 that restores Gerrit emails for some people that were incorrectly changed in CL 165639, with manual no-op addPerson line merges to address code review comments. The cherry-pick applied very cleanly with just two minor merge conflicts: one due to a Filippo's email already being fixed in CL 176037, and another due to a close new entry. Add tests to catch this from happening again, until the source of the problem is resolved in issue golang/go#34259. Updates golang/go#34259 Updates golang/go#28320 Updates golang/go#31919 Updates golang/go#27517 Updates golang/go#27631 Change-Id: Ia03a2b94403334d3f571ac5623e12d3bfd6f1e4f Reviewed-on: https://go-review.googlesource.com/c/build/+/195062 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is a cherry-pick of CL 135456 that restores Gerrit emails for some people that were incorrectly changed in CL 165639, with manual no-op addPerson line merges to address code review comments. The cherry-pick applied very cleanly with just two minor merge conflicts: one due to a Filippo's email already being fixed in CL 176037, and another due to a close new entry. Add tests to catch this from happening again, until the source of the problem is resolved in issue golang/go#34259. Updates golang/go#34259 Updates golang/go#28320 Updates golang/go#31919 Updates golang/go#27517 Updates golang/go#27631 Change-Id: Ia03a2b94403334d3f571ac5623e12d3bfd6f1e4f Reviewed-on: https://go-review.googlesource.com/c/build/+/195062 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
x/build/cmd/devapp/owners uses the first email returned by
gophers.GetPersonas the Gerrit email. Perhaps we should be more specific about this in the gophers package, but for now we should ensure that the first email is the Gerrit email. You cannot add a person to a review using their Gerrit ID email, for instance.The text was updated successfully, but these errors were encountered: