Skip to content

Commit 11b4857

Browse files
authored
Make review reminders only remind current reviewer (#12191)
1 parent 81a9344 commit 11b4857

File tree

2 files changed

+48
-11
lines changed

2 files changed

+48
-11
lines changed

.ci/magician/cmd/scheduled_pr_reminders.go

+22-10
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,12 @@ var scheduledPrReminders = &cobra.Command{
6666
return fmt.Errorf("did not provide GITHUB_TOKEN environment variable")
6767
}
6868
gh := github.NewClient(nil).WithAuthToken(githubToken)
69-
return execScheduledPrReminders(gh)
69+
mgh := membership.NewClient(githubToken)
70+
return execScheduledPrReminders(gh, mgh)
7071
},
7172
}
7273

73-
func execScheduledPrReminders(gh *github.Client) error {
74+
func execScheduledPrReminders(gh *github.Client, mgh GithubClient) error {
7475
ctx := context.Background()
7576
opt := &github.PullRequestListOptions{
7677
State: "open",
@@ -165,7 +166,14 @@ func execScheduledPrReminders(gh *github.Client) error {
165166
)
166167
sinceDays := businessDaysDiff(since, time.Now())
167168
if shouldNotify(pr, state, sinceDays) {
168-
comment, err := formatReminderComment(pr, state, sinceDays)
169+
// Determine the current primary reviewer.
170+
comments, err := mgh.GetPullRequestComments(fmt.Sprintf("%d", *pr.Number))
171+
if err != nil {
172+
return err
173+
}
174+
_, currentReviewer := membership.FindReviewerComment(comments)
175+
176+
reminderComment, err := formatReminderComment(pr, state, sinceDays, currentReviewer)
169177
if err != nil {
170178
fmt.Printf(
171179
"%d/%d: PR %d: error rendering comment: %s\n",
@@ -177,15 +185,15 @@ func execScheduledPrReminders(gh *github.Client) error {
177185
continue
178186
}
179187
if dryRun {
180-
fmt.Printf("DRY RUN: Would post comment: %s\n", comment)
188+
fmt.Printf("DRY RUN: Would post comment: %s\n", reminderComment)
181189
} else {
182190
_, _, err := gh.Issues.CreateComment(
183191
ctx,
184192
"GoogleCloudPlatform",
185193
"magic-modules",
186194
*pr.Number,
187195
&github.IssueComment{
188-
Body: github.String(comment),
196+
Body: github.String(reminderComment),
189197
},
190198
)
191199
if err != nil {
@@ -433,7 +441,7 @@ func shouldNotify(pr *github.PullRequest, state pullRequestReviewState, sinceDay
433441
return false
434442
}
435443

436-
func formatReminderComment(pullRequest *github.PullRequest, state pullRequestReviewState, sinceDays int) (string, error) {
444+
func formatReminderComment(pullRequest *github.PullRequest, state pullRequestReviewState, sinceDays int, currentReviewer string) (string, error) {
437445
embeddedTemplate := ""
438446
switch state {
439447
case waitingForMerge:
@@ -454,11 +462,15 @@ func formatReminderComment(pullRequest *github.PullRequest, state pullRequestRev
454462
panic(fmt.Sprintf("Unable to parse template for %s: %s", state.String(), err))
455463
}
456464

457-
coreReviewers := []string{}
458-
for _, reviewer := range pullRequest.RequestedReviewers {
459-
if membership.IsCoreReviewer(*reviewer.Login) {
460-
coreReviewers = append(coreReviewers, *reviewer.Login)
465+
var coreReviewers []string
466+
if currentReviewer == "" {
467+
for _, reviewer := range pullRequest.RequestedReviewers {
468+
if membership.IsCoreReviewer(*reviewer.Login) {
469+
coreReviewers = append(coreReviewers, *reviewer.Login)
470+
}
461471
}
472+
} else {
473+
coreReviewers = append(coreReviewers, currentReviewer)
462474
}
463475

464476
data := reminderCommentData{

.ci/magician/cmd/scheduled_pr_reminders_test.go

+26-1
Original file line numberDiff line numberDiff line change
@@ -794,6 +794,7 @@ func TestFormatReminderComment(t *testing.T) {
794794
sinceDays int
795795
expectedStrings []string
796796
notExpectedStrings []string
797+
currentReviewer string
797798
}{
798799
// waitingForMerge
799800
"waitingForMerge one week": {
@@ -933,6 +934,30 @@ func TestFormatReminderComment(t *testing.T) {
933934
"@other-reviewer",
934935
},
935936
},
937+
"waitingForReview three days with current reviewer": {
938+
pullRequest: &github.PullRequest{
939+
User: &github.User{Login: github.String("pr-author")},
940+
RequestedReviewers: []*github.User{
941+
&github.User{Login: github.String(firstCoreReviewer)},
942+
&github.User{Login: github.String(secondCoreReviewer)},
943+
&github.User{Login: github.String("other-reviewer")},
944+
},
945+
},
946+
state: waitingForReview,
947+
sinceDays: 3,
948+
expectedStrings: []string{
949+
"waiting for review for 3 weekdays",
950+
"disable-review-reminders",
951+
"@" + secondCoreReviewer,
952+
},
953+
notExpectedStrings: []string{
954+
"@GoogleCloudPlatform/terraform-team",
955+
"@pr-author",
956+
"@other-reviewer",
957+
"@" + firstCoreReviewer,
958+
},
959+
currentReviewer: secondCoreReviewer,
960+
},
936961

937962
// waitingForContributor
938963
"waitingForContributor two weeks": {
@@ -1048,7 +1073,7 @@ func TestFormatReminderComment(t *testing.T) {
10481073
t.Run(tn, func(t *testing.T) {
10491074
t.Parallel()
10501075

1051-
comment, err := formatReminderComment(tc.pullRequest, tc.state, tc.sinceDays)
1076+
comment, err := formatReminderComment(tc.pullRequest, tc.state, tc.sinceDays, tc.currentReviewer)
10521077
assert.Nil(t, err)
10531078

10541079
for _, s := range tc.expectedStrings {

0 commit comments

Comments
 (0)