feat: remove request review feature from cherrypicker (#631)

close #627

docs/en/plugins/cherrypicker.md

@@ -28,7 +28,6 @@ Note: **The above `resolve conflict` means that the tool will `git add` the conf
 In addition to implementing the core functionality of cherry-pick, it also supports a number of other features:
 
 - Use labels to mark which branches needs cherry-pick
-- Copy the reviewers of the current PR to the PR of cherry-pick 
 - Assign the PR of cherry-pick to the author or requester (the person who requested cherry-pick)
 - Copy the labels already added for the current PR
 

docs/plugins/cherrypicker.md

@@ -28,7 +28,6 @@ ti-community-cherrypicker 将帮助我们自动的 cherry-pick PR 的改动到
 除了实现 cherry-pick 的核心功能之外，它还支持了一些其他功能：
 
 - 使用 labels 来标记需要 cherry-pick 到哪些分支
-- 复制当前 PR 的 reviewers 到 cherry-pick 的 PR 
 - 将 cherry-pick 的 PR 分配给作者或者请求人（请求 cherry-pick 的人）
 - 复制当前 PR 已有的 labels
 

internal/pkg/externalplugins/cherrypicker/cherrypicker.go

@@ -16,7 +16,7 @@ limitations under the License.
 
 The original file of the code is at:
 https://github.com/kubernetes/test-infra/blob/master/prow/external-plugins/cherrypicker/server.go,
-which we modified to add support for copying the labels and reviewers.
+which we modified to add support for copying the labels.
 */
 
 package cherrypicker
@@ -59,7 +59,6 @@ const upstreamRemoteName = "upstream"
 type githubClient interface {
 	AddLabels(org, repo string, number int, labels ...string) error
 	AssignIssue(org, repo string, number int, logins []string) error
-	RequestReview(org, repo string, number int, logins []string) error
 	CreateComment(org, repo string, number int, comment string) error
 	CreateFork(org, repo string) (string, error)
 	CreatePullRequest(org, repo, title, body, head, base string, canModify bool) (int, error)
@@ -683,19 +682,6 @@ func (s *Server) handle(logger *logrus.Entry, requestor string,
 		logger.WithError(err).Warnf("Failed to add labels %v", labels.List())
 	}
 
-	// Copying original pull request reviewers.
-	var reviewers []string
-	for _, reviewer := range pr.RequestedReviewers {
-		reviewers = append(reviewers, reviewer.Login)
-	}
-	if err := s.GitHubClient.RequestReview(org, repo, createdNum, reviewers); err != nil {
-		logger.WithError(err).Warn("failed to request review to new PR")
-		// Ignore returning errors on failure to request review as this is likely
-		// due to users not being members of the org so that they can't be requested
-		// in PRs.
-		return nil
-	}
-
 	// Assign pull request to requestor.
 	if err := s.GitHubClient.AssignIssue(org, repo, createdNum, []string{requestor}); err != nil {
 		logger.WithError(err).Warn("failed to assign to new PR")

internal/pkg/externalplugins/cherrypicker/cherrypicker_test.go

@@ -16,7 +16,7 @@ limitations under the License.
 
 The original file of the code is at:
 https://github.com/kubernetes/test-infra/blob/master/prow/external-plugins/cherrypicker/server_test.go,
-which we modified to add support for copying the labels and reviewers.
+which we modified to add support for copying the labels.
 */
 
 package cherrypicker
@@ -84,21 +84,6 @@ func (f *fghc) AssignIssue(org, repo string, number int, logins []string) error
 	return nil
 }
 
-func (f *fghc) RequestReview(org, repo string, number int, logins []string) error {
-	f.Lock()
-	defer f.Unlock()
-	var users []github.User
-	for _, login := range logins {
-		users = append(users, github.User{Login: login})
-	}
-	for i := range f.prs {
-		if number == f.prs[i].Number {
-			f.prs[i].RequestedReviewers = users
-		}
-	}
-	return nil
-}
-
 func (f *fghc) GetPullRequest(org, repo string, number int) (*github.PullRequest, error) {
 	f.Lock()
 	defer f.Unlock()
@@ -146,24 +131,19 @@ func (f *fghc) EnsureFork(forkingUser, org, repo string) (string, error) {
 	return repo, nil
 }
 
-var prFmt = `title=%q body=%q head=%s base=%s labels=%v reviewers=%v assignees=%v`
+var prFmt = `title=%q body=%q head=%s base=%s labels=%v assignees=%v`
 
 func prToString(pr github.PullRequest) string {
 	var labels []string
 	for _, label := range pr.Labels {
 		labels = append(labels, label.Name)
 	}
 
-	var reviewers []string
-	for _, reviewer := range pr.RequestedReviewers {
-		reviewers = append(reviewers, reviewer.Login)
-	}
-
 	var assignees []string
 	for _, assignee := range pr.Assignees {
 		assignees = append(assignees, assignee.Login)
 	}
-	return fmt.Sprintf(prFmt, pr.Title, pr.Body, pr.Head.Ref, pr.Base.Ref, labels, reviewers, assignees)
+	return fmt.Sprintf(prFmt, pr.Title, pr.Body, pr.Head.Ref, pr.Base.Ref, labels, assignees)
 }
 
 func (f *fghc) CreateIssue(org, repo, title, body string, milestone int, labels, assignees []string) (int, error) {
@@ -315,11 +295,6 @@ func testCherryPickIC(clients localgit.Clients, t *testing.T) {
 			Merged: true,
 			Title:  "This is a fix for X",
 			Body:   body,
-			RequestedReviewers: []github.User{
-				{
-					Login: "user1",
-				},
-			},
 			Assignees: []github.User{
 				{
 					Login: "user2",
@@ -364,12 +339,8 @@ func testCherryPickIC(clients localgit.Clients, t *testing.T) {
 			expectedLabels = append(expectedLabels, label.Name)
 		}
 		expectedLabels = append(expectedLabels, "type/cherrypick-for-"+branch)
-		var expectedReviewers []string
-		for _, reviewer := range ghc.pr.RequestedReviewers {
-			expectedReviewers = append(expectedReviewers, reviewer.Login)
-		}
 		return fmt.Sprintf(prFmt, expectedTitle, expectedBody, expectedHead,
-			branch, expectedLabels, expectedReviewers, expectedAssignees)
+			branch, expectedLabels, expectedAssignees)
 	}
 
 	getSecret := func() []byte {
@@ -484,11 +455,6 @@ func testCherryPickPRWithLabels(clients localgit.Clients, t *testing.T) {
 			Merged:   true,
 			MergeSHA: new(string),
 			Title:    "This is a fix for Y",
-			RequestedReviewers: []github.User{
-				{
-					Login: "user1",
-				},
-			},
 		},
 	}
 
@@ -595,13 +561,9 @@ func testCherryPickPRWithLabels(clients localgit.Clients, t *testing.T) {
 					}
 				}
 				expectedLabels = append(expectedLabels, "type/cherrypick-for-"+branch)
-				var expectedReviewers []string
-				for _, reviewer := range pr.PullRequest.RequestedReviewers {
-					expectedReviewers = append(expectedReviewers, reviewer.Login)
-				}
 				expectedAssignees := []string{"developer"}
 				return fmt.Sprintf(prFmt, expectedTitle, expectedBody, expectedHead,
-					branch, expectedLabels, expectedReviewers, expectedAssignees)
+					branch, expectedLabels, expectedAssignees)
 			}
 
 			expectedPRsNum := 2
@@ -722,11 +684,6 @@ func testCherryPickPRWithComment(clients localgit.Clients, t *testing.T) {
 						Name: "test",
 					},
 				},
-				RequestedReviewers: []github.User{
-					{
-						Login: "user1",
-					},
-				},
 				Assignees: []github.User{
 					{
 						Login: "approver",
@@ -759,11 +716,6 @@ func testCherryPickPRWithComment(clients localgit.Clients, t *testing.T) {
 					Name: "test",
 				},
 			},
-			RequestedReviewers: []github.User{
-				{
-					Login: "user1",
-				},
-			},
 			Assignees: []github.User{
 				{
 					Login: "approver",
@@ -811,13 +763,9 @@ func testCherryPickPRWithComment(clients localgit.Clients, t *testing.T) {
 		for _, label := range pr.PullRequest.Labels {
 			expectedLabels = append(expectedLabels, label.Name)
 		}
-		var expectedReviewers []string
-		for _, reviewer := range pr.PullRequest.RequestedReviewers {
-			expectedReviewers = append(expectedReviewers, reviewer.Login)
-		}
 		expectedAssignees := []string{"approver"}
 		return fmt.Sprintf(prFmt, expectedTitle, expectedBody, expectedHead,
-			branch, expectedLabels, expectedReviewers, expectedAssignees)
+			branch, expectedLabels, expectedAssignees)
 	}
 
 	if len(ghc.prs) != len(expectedBranches) {
@@ -901,11 +849,6 @@ func testCherryPickPRLabeled(clients localgit.Clients, t *testing.T) {
 				Merged:   true,
 				MergeSHA: new(string),
 				Title:    "This is a fix for Y",
-				RequestedReviewers: []github.User{
-					{
-						Login: "user1",
-					},
-				},
 			},
 			Label: label,
 		}
@@ -1043,13 +986,9 @@ func testCherryPickPRLabeled(clients localgit.Clients, t *testing.T) {
 							expectedLabels = append(expectedLabels, label.Name)
 						}
 						expectedLabels = append(expectedLabels, "type/cherrypick-for-"+branch)
-						var expectedReviewers []string
-						for _, reviewer := range pr(lb).PullRequest.RequestedReviewers {
-							expectedReviewers = append(expectedReviewers, reviewer.Login)
-						}
 						expectedAssignees := []string{"developer"}
 						return fmt.Sprintf(prFmt, expectedTitle, expectedBody, expectedHead,
-							branch, expectedLabels, expectedReviewers, expectedAssignees)
+							branch, expectedLabels, expectedAssignees)
 					}
 
 					if tc.shouldToggle {