enhance github mix report type

config/config.yaml → config/config.example.yaml

@@ -1,11 +1,12 @@
+# 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.
 # 2. Modifying the default global configuration.
 # 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"

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"

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:

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:

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,15 +78,15 @@ 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
 				}
 
 				// 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,14 +105,14 @@ 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)
 			}
 		}
 	}
 }
 
 const ReferenceFooter = `
 <details>
-<summary>详细解释</summary>
+<summary>Details</summary>
 %s
 </details>`

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

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)
 }

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]** <sub>reported by [reviewbot](https://github.com/qiniu/reviewbot):cow:</sub>\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
+}