From acfc97fe5e075e024cdde0cef652e0cd2c6116fe Mon Sep 17 00:00:00 2001 From: Mini256 Date: Tue, 14 Dec 2021 17:36:36 +0800 Subject: [PATCH] feat: lgtm support ignore the prompt when approval is invalid (#827) close #743 --- docs/en/plugins/lgtm.md | 12 +++--- docs/plugins/lgtm.md | 10 +++-- internal/pkg/externalplugins/config.go | 2 + internal/pkg/externalplugins/lgtm/lgtm.go | 47 +++++++++++++++-------- 4 files changed, 45 insertions(+), 26 deletions(-) diff --git a/docs/en/plugins/lgtm.md b/docs/en/plugins/lgtm.md index c86ad92b8..dce5b899d 100644 --- a/docs/en/plugins/lgtm.md +++ b/docs/en/plugins/lgtm.md @@ -34,12 +34,13 @@ This feature is triggered in the following cases: - Use Approve/Request Changes feature of GitHub -## Parameter Configuration +## Parameter Configuration -| Parameter Name | Type | Description | -| -------------------- | -------- | --------------------------------------------------------------------------- | -| repos | []string | Repositories | -| pull_owners_endpoint | string | PR owners RESTFUL API | +| Parameter Name | Type | Description | +|------------------------------|----------|-----------------------------------| +| repos | []string | Repositories | +| pull_owners_endpoint | string | PR owners RESTFUL API | +| ignore_invalid_review_prompt | bool | Do not prompt for invalid reviews | For example: @@ -52,6 +53,7 @@ ti-community-lgtm: - ti-community-infra/ti-challenge-bot - tikv/pd pull_owners_endpoint: https://prow.tidb.io/ti-community-owners # You can define different URL to get owners + ignore_invalid_review_prompt: true ``` ## Reference Documents diff --git a/docs/plugins/lgtm.md b/docs/plugins/lgtm.md index 070710b43..c017bd88f 100644 --- a/docs/plugins/lgtm.md +++ b/docs/plugins/lgtm.md @@ -36,10 +36,11 @@ ti-community-lgtm 是会根据命令和权限自动的为 PR 添加 LGTM 对应 ## 参数配置 -| 参数名 | 类型 | 说明 | -| -------------------- | -------- | ----------------------------------------------------------------- | -| repos | []string | 配置生效仓库 | -| pull_owners_endpoint | string | PR owners RESTFUL 接口地址 | +| 参数名 | 类型 | 说明 | +|------------------------------|----------|------------------------| +| repos | []string | 配置生效仓库 | +| pull_owners_endpoint | string | PR owners RESTFUL 接口地址 | +| ignore_invalid_review_prompt | bool | 不对无效的 review 进行提示 | 例如： @@ -52,6 +53,7 @@ ti-community-lgtm: - ti-community-infra/ti-challenge-bot - tikv/pd pull_owners_endpoint: https://prow.tidb.io/ti-community-owners # 你可以定义不同的获取 owners 的链接 + ignore_invalid_review_prompt: true ``` ## 参考文档 diff --git a/internal/pkg/externalplugins/config.go b/internal/pkg/externalplugins/config.go index d2dbda859..a78567eca 100644 --- a/internal/pkg/externalplugins/config.go +++ b/internal/pkg/externalplugins/config.go @@ -62,6 +62,8 @@ type TiCommunityLgtm struct { Repos []string `json:"repos,omitempty"` // PullOwnersEndpoint specifies the URL of the reviewer of pull request. PullOwnersEndpoint string `json:"pull_owners_endpoint,omitempty"` + // IgnoreInvalidReviewPrompt specifies no prompt when review is invalid, default is `false`. + IgnoreInvalidReviewPrompt bool `json:"ignore_invalid_review_prompt"` } // TiCommunityMerge specifies a configuration for a single merge. diff --git a/internal/pkg/externalplugins/lgtm/lgtm.go b/internal/pkg/externalplugins/lgtm/lgtm.go index 499c9fc04..1d0cfe48c 100644 --- a/internal/pkg/externalplugins/lgtm/lgtm.go +++ b/internal/pkg/externalplugins/lgtm/lgtm.go @@ -152,7 +152,7 @@ func handle(wantLGTM bool, config *tiexternalplugins.Configuration, rc reviewCtx log.WithField("duration", time.Since(funcStart).String()).Debug("Completed handle") }() - author := rc.author + currentReviewer := rc.author number := rc.number body := rc.body htmlURL := rc.htmlURL @@ -176,19 +176,25 @@ func handle(wantLGTM bool, config *tiexternalplugins.Configuration, rc reviewCtx } // Not reviewers but want to add LGTM. - if !reviewers.Has(author) && wantLGTM { + if !reviewers.Has(currentReviewer) && wantLGTM { resp := "Thanks for your review. " resp += "The bot only counts approvals from reviewers and higher roles in [list](" + tichiURL + "), " resp += "but you're still welcome to leave your comments." log.Infof("Reply approve pull request in comment: \"%s\"", resp) - return gc.CreateComment(org, repo, number, tiexternalplugins.FormatResponseRaw(body, htmlURL, author, resp)) + if !opts.IgnoreInvalidReviewPrompt { + return gc.CreateComment(org, repo, number, tiexternalplugins.FormatResponseRaw(body, htmlURL, currentReviewer, resp)) + } + return nil } // Not reviewers but want to remove LGTM. - if !reviewers.Has(author) && !wantLGTM { + if !reviewers.Has(currentReviewer) && !wantLGTM { resp := "Request changes is only allowed for the reviewers in [list](" + tichiURL + ")." log.Infof("Reply request changes pull request in comment: \"%s\"", resp) - return gc.CreateComment(org, repo, number, tiexternalplugins.FormatResponseRaw(body, htmlURL, author, resp)) + if !opts.IgnoreInvalidReviewPrompt { + return gc.CreateComment(org, repo, number, tiexternalplugins.FormatResponseRaw(body, htmlURL, currentReviewer, resp)) + } + return nil } labels, err := gc.GetIssueLabels(org, repo, number) @@ -250,13 +256,13 @@ func handle(wantLGTM bool, config *tiexternalplugins.Configuration, rc reviewCtx } else if nextLabel != "" && wantLGTM { reviewedReviewers := getReviewersFromNotification(latestNotification) // Ignore already reviewed reviewer. - if reviewedReviewers.Has(author) { - log.Infof("Ignore %s's multiple reviews.", author) + if reviewedReviewers.Has(currentReviewer) { + log.Infof("Ignore %s's multiple reviews.", currentReviewer) return nil } - // Add author as reviewers and create new notification. - reviewedReviewers.Insert(author) + // Add currentReviewer as reviewers and create new notification. + reviewedReviewers.Insert(currentReviewer) newMsg, err := getMessage(reviewedReviewers.List(), config.CommandHelpLink, config.PRProcessLink, tichiURL, org, repo) if err != nil { return err @@ -275,14 +281,7 @@ func handle(wantLGTM bool, config *tiexternalplugins.Configuration, rc reviewCtx } } - log.Info("Adding LGTM label.") - // Remove current label. - if currentLabel != "" { - if err := gc.RemoveLabel(org, repo, number, currentLabel); err != nil { - return err - } - } - if err := gc.AddLabel(org, repo, number, nextLabel); err != nil { + if err := updateLabels(gc, log, org, repo, number, currentLabel, nextLabel); err != nil { return err } @@ -313,6 +312,20 @@ func getCurrentAndNextLabel(prefix string, labels []github.Label, needsLgtm int) return currentLabel, nextLabel } +func updateLabels(gc githubClient, log *logrus.Entry, org, repo string, number int, + currentLabel, nextLabel string) error { + log.Info("Adding LGTM label.") + + // Remove current label. + if currentLabel != "" { + if err := gc.RemoveLabel(org, repo, number, currentLabel); err != nil { + return err + } + } + + return gc.AddLabel(org, repo, number, nextLabel) +} + // getReviewersFromNotification get the reviewers from latest notification. func getReviewersFromNotification(latestNotification *github.IssueComment) sets.String { result := sets.String{}