diff --git a/config/config.yaml b/config/config.example.yaml
similarity index 96%
rename from config/config.yaml
rename to config/config.example.yaml
index 6e461021..1014d077 100644
--- a/config/config.yaml
+++ b/config/config.example.yaml
@@ -1,3 +1,4 @@
+# This is an example of the config file for reviewbot.
# Reviewbot follows a zero-configuration approach in its design, where the default behavior implemented in the code should be suitable for the majority of scenarios.
# Configuration of the file is only necessary in special cases, such as:
# 1. Using specific commands for a particular repository.
@@ -5,7 +6,7 @@
# 3. Conducting specific gray testing for a particular feature.
globalDefaultConfig: # global default settings, will be overridden by qbox org and repo specific settings if they exist
- githubReportType: "github_check_run" # github_pr_review, github_check_run
+ githubReportType: "github_mix" # github_pr_review, github_check_run, github_mix
golangcilintConfig: "config/linters-config/.golangci.yml" # golangci-lint config file to use
copySSHKeyToContainer: "/root/.ssh/id_rsa"
@@ -138,7 +139,7 @@ customConfig: # custom config for specific orgs or repos
issueReferences:
golangci-lint:
- - pattern: 'ST1003'
+ - pattern: "ST1003"
url: "https://github.com/qiniu/reviewbot/issues/398"
- pattern: '^do not define dynamic errors, use wrapped static errors instead:.*\(err113\)$'
url: "https://github.com/qiniu/reviewbot/issues/418"
diff --git a/config/config.go b/config/config.go
index b2be7068..6668d7be 100644
--- a/config/config.go
+++ b/config/config.go
@@ -379,6 +379,8 @@ const (
GithubCheckRuns ReportType = "github_check_run"
GithubPRReview ReportType = "github_pr_review"
// GithubMixType is the type of the report that mix the github_check_run and github_pr_review.
+ // which use the github_check_run to report all lint results as a check run summary,
+ // but use the github_pr_review to report top 10 lint results to pull request review comments at most.
GithubMixType ReportType = "github_mix"
// for debug and testing.
Quiet ReportType = "quiet"
diff --git a/config/linters-config/.golangci.goplus.yml b/config/linters-config/.golangci.goplus.yml
index 9b88a22f..33cbc21b 100644
--- a/config/linters-config/.golangci.goplus.yml
+++ b/config/linters-config/.golangci.goplus.yml
@@ -39,6 +39,9 @@ linters-settings:
disabled: true
- name: cyclomatic
arguments: [50]
+ # revive 的 error-return 误报太多, 本身已有 errcheck 检查
+ - name: error-return
+ disabled: true
errcheck:
exclude-functions:
diff --git a/config/linters-config/.golangci.yml b/config/linters-config/.golangci.yml
index 5a16c633..31f95e59 100644
--- a/config/linters-config/.golangci.yml
+++ b/config/linters-config/.golangci.yml
@@ -37,6 +37,9 @@ linters-settings:
disabled: true
- name: cyclomatic
arguments: [30]
+ # revive 的 error-return 误报太多, 本身已有 errcheck 检查
+ - name: error-return
+ disabled: true
errcheck:
exclude-functions:
diff --git a/internal/linters/agent.go b/internal/linters/agent.go
index 11b412d9..33207a48 100644
--- a/internal/linters/agent.go
+++ b/internal/linters/agent.go
@@ -55,8 +55,8 @@ type Agent struct {
IssueReferences []config.CompiledIssueReference
}
-// ApplyIssueReferences applies the issue references to the lint results.
-func (a *Agent) ApplyIssueReferences(ctx context.Context, lintResults map[string][]LinterOutput) {
+// ApplyTypedMessageByIssueReferences applies the issue references to the lint results with the typed message.
+func (a *Agent) ApplyTypedMessageByIssueReferences(ctx context.Context, lintResults map[string][]LinterOutput) {
log := lintersutil.FromContext(ctx)
var msgFormat string
format := a.LinterConfig.ReportFormat
@@ -78,7 +78,7 @@ func (a *Agent) ApplyIssueReferences(ctx context.Context, lintResults map[string
if !ref.Pattern.MatchString(o.Message) {
continue
}
- lintResults[file][i].Message = fmt.Sprintf(msgFormat, o.Message, ref.URL)
+ lintResults[file][i].TypedMessage = fmt.Sprintf(msgFormat, o.TypedMessage, ref.URL)
if format != config.GithubPRReview && format != config.GithubMixType {
continue
@@ -86,7 +86,7 @@ func (a *Agent) ApplyIssueReferences(ctx context.Context, lintResults map[string
// specific for github pr review format
if issueContent, ok := issueCache.Get(ref.URL); ok && !issueCache.IsExpired(ref.URL) {
- lintResults[file][i].Message += fmt.Sprintf(ReferenceFooter, issueContent)
+ lintResults[file][i].TypedMessage += fmt.Sprintf(ReferenceFooter, issueContent)
continue
}
@@ -105,7 +105,7 @@ func (a *Agent) ApplyIssueReferences(ctx context.Context, lintResults map[string
issueContent := issue.GetBody()
issueCache.Set(ref.URL, issueContent)
- lintResults[file][i].Message += fmt.Sprintf(ReferenceFooter, issueContent)
+ lintResults[file][i].TypedMessage += fmt.Sprintf(ReferenceFooter, issueContent)
}
}
}
@@ -113,6 +113,6 @@ func (a *Agent) ApplyIssueReferences(ctx context.Context, lintResults map[string
const ReferenceFooter = `
-详细解释
+Details
%s
`
diff --git a/internal/linters/git-flow/commit/commit.go b/internal/linters/git-flow/commit/commit.go
index 87dfdaeb..66ef3cf1 100644
--- a/internal/linters/git-flow/commit/commit.go
+++ b/internal/linters/git-flow/commit/commit.go
@@ -146,7 +146,7 @@ func handle(ctx context.Context, agent linters.Agent, org, repo, author string,
if err != nil {
return err
}
- log.Infof("create comment success: %v", c)
+ log.Infof("create comment success: %v", c.HTMLURL)
}
return nil
diff --git a/internal/linters/linters.go b/internal/linters/linters.go
index 6bf6940e..3343c63b 100644
--- a/internal/linters/linters.go
+++ b/internal/linters/linters.go
@@ -89,10 +89,12 @@ type LinterOutput struct {
Line int
// Column is the Column number
Column int
- // Message is the staticcheck Message
+ // Message is the linter message
Message string
// StartLine required when using multi-line comments
StartLine int
+ // TypedMessage is the typed message
+ TypedMessage string
}
const CommentFooter = `
@@ -192,12 +194,11 @@ func Report(ctx context.Context, a Agent, lintResults map[string][]LinterOutput)
log.Infof("[%s] found %d files with valid %d linter errors related to this PR %d (%s) \n", linterName, len(lintResults), countLinterErrors(lintResults), num, orgRepo)
- a.ApplyIssueReferences(ctx, lintResults)
+ a.ApplyTypedMessageByIssueReferences(ctx, lintResults)
if len(lintResults) > 0 {
metric.IncIssueCounter(orgRepo, linterName, a.Provider.GetCodeReviewInfo().URL, a.Provider.GetCodeReviewInfo().HeadSHA, float64(countLinterErrors(lintResults)))
}
- log.Infof("[%s] lint results: %v", linterName, lintResults)
return a.Provider.Report(ctx, a, lintResults)
}
diff --git a/internal/linters/providergithub.go b/internal/linters/providergithub.go
index 74925207..faac9a3d 100644
--- a/internal/linters/providergithub.go
+++ b/internal/linters/providergithub.go
@@ -127,15 +127,22 @@ func RetryWithBackoff(ctx context.Context, f func() error) error {
}
func linterNamePrefix(linterName string) string {
- return fmt.Sprintf("**[%s]** reported by [reviewbot](https://github.com/qiniu/reviewbot):cow:\n", linterName)
+ return fmt.Sprintf("**[%s]** reported by [reviewbot](https://github.com/qiniu/reviewbot):cow:\n", linterName)
}
func constructPullRequestComments(linterOutputs map[string][]LinterOutput, linterName, commitID string) []*github.PullRequestComment {
var comments []*github.PullRequestComment
for file, outputs := range linterOutputs {
for _, output := range outputs {
- message := fmt.Sprintf("%s %s\n%s",
- linterName, output.Message, CommentFooter)
+ // use the typed message as first priority
+ var message string
+ if output.TypedMessage != "" {
+ message = fmt.Sprintf("%s %s",
+ linterName, output.TypedMessage)
+ } else {
+ message = fmt.Sprintf("%s %s",
+ linterName, output.Message)
+ }
if output.StartLine != 0 {
comments = append(comments, &github.PullRequestComment{
@@ -249,6 +256,7 @@ func (g *GithubProvider) HandleComments(ctx context.Context, outputs map[string]
}
func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[string][]LinterOutput) error {
+ log := lintersutil.FromContext(ctx)
linterName := a.LinterConfig.Name
org := a.Provider.GetCodeReviewInfo().Org
repo := a.Provider.GetCodeReviewInfo().Repo
@@ -257,7 +265,8 @@ func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[st
switch a.LinterConfig.ReportFormat {
case config.GithubCheckRuns:
- ch, err := g.CreateGithubChecks(ctx, a, lintResults)
+ check := newBaseCheckRun(a, lintResults)
+ ch, err := g.CreateCheckRun(ctx, org, repo, check)
if err != nil {
if !errors.Is(err, context.Canceled) {
log.Errorf("failed to create github checks: %v", err)
@@ -287,7 +296,8 @@ func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[st
metric.NotifyWebhookByText(ConstructGotchaMsg(linterName, a.Provider.GetCodeReviewInfo().URL, addedCmts[0].GetHTMLURL(), lintResults))
case config.GithubMixType:
// report all lint results as a check run summary, but not annotations
- ch, err := g.CreateGithubChecks(ctx, a, lintResults)
+ check := newMixCheckRun(a, lintResults)
+ ch, err := g.CreateCheckRun(ctx, org, repo, check)
if err != nil {
if !errors.Is(err, context.Canceled) {
log.Errorf("failed to create github checks: %v", err)
@@ -424,7 +434,7 @@ func (g *GithubProvider) DeletePullReviewComments(ctx context.Context, owner, re
return err
}
- log.Infof("delete comment success: %v", comment)
+ log.Infof("delete comment success: %v", comment.GetHTMLURL())
}
return nil
@@ -446,71 +456,20 @@ func (g *GithubProvider) CreatePullReviewComments(ctx context.Context, owner str
log.Errorf("create comment failed: %v", resp)
return ErrCreateComment
}
+ log.Infof("create comment success: %v", cm.GetHTMLURL())
addedComments = append(addedComments, cm)
return nil
})
if err != nil {
return nil, err
}
-
- log.Infof("create comment success: %v", comment)
}
return addedComments, nil
}
-// CreateGithubChecks creates github checks for the specified pull request.
-func (g *GithubProvider) CreateGithubChecks(ctx context.Context, a Agent, lintErrs map[string][]LinterOutput) (*github.CheckRun, error) {
- var (
- headSha = a.Provider.GetCodeReviewInfo().HeadSHA
- owner = a.Provider.GetCodeReviewInfo().Org
- repo = a.Provider.GetCodeReviewInfo().Repo
- startTime = a.Provider.GetCodeReviewInfo().UpdatedAt
- linterName = a.LinterConfig.Name
- )
- log := lintersutil.FromContext(ctx)
- annotations := toGithubCheckRunAnnotations(lintErrs)
- // limit the number of annotations to 50
- // see: https://github.com/qiniu/reviewbot/issues/258
- if len(annotations) > 50 {
- annotations = annotations[:50]
- }
- check := github.CreateCheckRunOptions{
- Name: linterName,
- HeadSHA: headSha,
- Status: github.String("completed"),
- StartedAt: &github.Timestamp{
- Time: startTime,
- },
- CompletedAt: &github.Timestamp{
- Time: time.Now(),
- },
- Output: &github.CheckRunOutput{
- Title: github.String(fmt.Sprintf("%s found %d issues related to your changes", linterName, len(annotations))),
- Annotations: annotations,
- },
- }
-
- logURL := a.GenLogViewURL()
- if logURL == "" {
- check.Output.Summary = github.String(Reference)
- } else {
- log.Debugf("Log view :%s", logURL)
- check.Output.Summary = github.String(fmt.Sprintf("This is [the detailed log](%s).\n\n%s", logURL, Reference))
- }
-
- if len(annotations) > 0 {
- check.Conclusion = github.String("failure")
- } else {
- check.Conclusion = github.String("success")
- }
-
- if a.LinterConfig.ReportFormat == config.GithubMixType {
- check.Output.Title = github.String(fmt.Sprintf("[%s] found %d issues related to your changes", linterName, len(annotations)))
- check.Output.Annotations = nil
- check.Output.Summary = github.String("All issues are be shown in the following table \n" + printAllLintResultswithMarkdown(lintErrs))
- }
-
+// CreateCheckRun submits the check run to GitHub.
+func (g *GithubProvider) CreateCheckRun(ctx context.Context, owner string, repo string, check github.CreateCheckRunOptions) (*github.CheckRun, error) {
var ch *github.CheckRun
err := RetryWithBackoff(ctx, func() error {
checkRun, resp, err := g.GithubClient.Checks.CreateCheckRun(ctx, owner, repo, check)
@@ -533,23 +492,6 @@ func (g *GithubProvider) CreateGithubChecks(ctx context.Context, a Agent, lintEr
return ch, err
}
-func printAllLintResultswithMarkdown(lintErrs map[string][]LinterOutput) string {
- info := markdownTableHeader
- for file, outputs := range lintErrs {
- for _, output := range outputs {
- // It's not necessary to show the details, just show the error line is enough.
- outMsg := strings.Split(output.Message, "\n")[0]
- info += fmt.Sprintf("| %s:%d | %s |\n", file, output.Line, outMsg)
- }
- }
- return info
-}
-
-var markdownTableHeader = `
-| filepath | linter error |
-|-------------------|-------------------------------|
-`
-
func (g *GithubProvider) ListCommits(ctx context.Context, org, repo string, number int) ([]Commit, error) {
log := lintersutil.FromContext(ctx)
opts := &github.ListOptions{
@@ -689,3 +631,89 @@ func (g *GithubProvider) ProcessNeedToAddComments(ctx context.Context, a Agent,
comments := constructPullRequestComments(toAdds, linterNamePrefix(linterName), a.Provider.GetCodeReviewInfo().HeadSHA)
return comments, nil
}
+
+// newBaseCheckRun creates the base check run options.
+func newBaseCheckRun(a Agent, lintErrs map[string][]LinterOutput) github.CreateCheckRunOptions {
+ var (
+ headSha = a.Provider.GetCodeReviewInfo().HeadSHA
+ startTime = a.Provider.GetCodeReviewInfo().UpdatedAt
+ linterName = a.LinterConfig.Name
+ )
+
+ annotations := toGithubCheckRunAnnotations(lintErrs)
+ if len(annotations) > 50 {
+ annotations = annotations[:50]
+ }
+
+ check := github.CreateCheckRunOptions{
+ Name: linterName,
+ HeadSHA: headSha,
+ Status: github.String("completed"),
+ StartedAt: &github.Timestamp{
+ Time: startTime,
+ },
+ CompletedAt: &github.Timestamp{
+ Time: time.Now(),
+ },
+ Output: &github.CheckRunOutput{
+ Title: github.String(fmt.Sprintf("%s found %d issues related to your changes", linterName, len(annotations))),
+ Annotations: annotations,
+ },
+ }
+
+ logURL := a.GenLogViewURL()
+ if logURL == "" {
+ check.Output.Summary = github.String(Reference)
+ } else {
+ log.Debugf("Log view :%s", logURL)
+ check.Output.Summary = github.String(fmt.Sprintf("This is [the detailed log](%s).\n\n%s", logURL, Reference))
+ }
+
+ if len(annotations) > 0 {
+ check.Conclusion = github.String("failure")
+ } else {
+ check.Conclusion = github.String("success")
+ }
+
+ return check
+}
+
+func newMixCheckRun(a Agent, lintErrs map[string][]LinterOutput) github.CreateCheckRunOptions {
+ check := newBaseCheckRun(a, lintErrs)
+ if len(lintErrs) == 0 {
+ // if no lint errors, just return the base check run
+ return check
+ }
+
+ // delete annotations since it will use PR review comments to show the details in mix report type
+ check.Output.Annotations = nil
+
+ // but still use details to all linter outputs
+ var b strings.Builder
+ // Add title and description
+ b.WriteString("## 🔍 Check Results Details\n\n")
+ b.WriteString("The following shows all issues found related to your changes:\n\n")
+
+ // Group results by file
+ b.WriteString("```text\n")
+ for file, outputs := range lintErrs {
+ for _, output := range outputs {
+ b.WriteString(fmt.Sprintf("%s:%d: %s\n", file, output.Line, output.Message))
+ }
+ }
+ b.WriteString("```\n")
+
+ // Add action guide
+ b.WriteString("\n### 🔄 How to Handle?\n\n")
+ b.WriteString("1. Fix the issues above and submit your code again\n")
+ b.WriteString("2. Or click the `Re-run` button to run the check again\n")
+ b.WriteString("3. If you think this is a false positive, please contact your support team\n\n")
+
+ // Add notes
+ b.WriteString("### ℹ️ Notes\n\n")
+ b.WriteString("- To avoid too many comments, only the top 10 issues will be shown in PR comments\n")
+ b.WriteString("- For any other issues, feel free to create an issue in [reviewbot](https://github.com/qiniu/reviewbot) repository\n")
+
+ check.Output.Text = github.String(b.String())
+ return check
+}