From be070ca4adfcfbfadba72bf34a30fe5297709894 Mon Sep 17 00:00:00 2001 From: Mini256 Date: Fri, 9 Jul 2021 15:53:31 +0800 Subject: [PATCH] feat: remove request review feature from cherrypicker (#631) close #627 --- docs/en/plugins/cherrypicker.md | 1 - docs/plugins/cherrypicker.md | 1 - .../cherrypicker/cherrypicker.go | 16 +--- .../cherrypicker/cherrypicker_test.go | 75 ++----------------- 4 files changed, 8 insertions(+), 85 deletions(-) diff --git a/docs/en/plugins/cherrypicker.md b/docs/en/plugins/cherrypicker.md index fb40640aa..f6e7c2331 100644 --- a/docs/en/plugins/cherrypicker.md +++ b/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 diff --git a/docs/plugins/cherrypicker.md b/docs/plugins/cherrypicker.md index cc9b5d7a4..d1d70aaf7 100644 --- a/docs/plugins/cherrypicker.md +++ b/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 diff --git a/internal/pkg/externalplugins/cherrypicker/cherrypicker.go b/internal/pkg/externalplugins/cherrypicker/cherrypicker.go index 2cdc3df1f..8af69f642 100644 --- a/internal/pkg/externalplugins/cherrypicker/cherrypicker.go +++ b/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") diff --git a/internal/pkg/externalplugins/cherrypicker/cherrypicker_test.go b/internal/pkg/externalplugins/cherrypicker/cherrypicker_test.go index 225811eb6..3d0fe26ba 100644 --- a/internal/pkg/externalplugins/cherrypicker/cherrypicker_test.go +++ b/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,7 +131,7 @@ 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 @@ -154,16 +139,11 @@ func prToString(pr github.PullRequest) string { 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 {