Skip to content

Commit

Permalink
feat: lgtm support ignore the prompt when approval is invalid (#827)
Browse files Browse the repository at this point in the history
close #743
  • Loading branch information
Mini256 authored Dec 14, 2021
1 parent a4adcd0 commit acfc97f
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 26 deletions.
12 changes: 7 additions & 5 deletions docs/en/plugins/lgtm.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand All @@ -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
Expand Down
10 changes: 6 additions & 4 deletions docs/plugins/lgtm.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 进行提示 |

例如:

Expand All @@ -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
```
## 参考文档
Expand Down
2 changes: 2 additions & 0 deletions internal/pkg/externalplugins/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
47 changes: 30 additions & 17 deletions internal/pkg/externalplugins/lgtm/lgtm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down Expand Up @@ -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{}
Expand Down

0 comments on commit acfc97f

Please sign in to comment.