Merge pull request #1037 from cloudflare/github

Refactor github reporer to use common comments code

cmd/pint/ci.go

@@ -171,7 +171,7 @@ func actionCI(c *cli.Context) error {
  ); err != nil {
  return err
  }
- reps = append(reps, gl)
+ reps = append(reps, reporter.NewCommentReporter(gl))
  }
 
  meta.cfg.Repository = detectRepository(meta.cfg.Repository)
@@ -191,6 +191,12 @@ func actionCI(c *cli.Context) error {
  return fmt.Errorf("got not a valid number via GITHUB_PULL_REQUEST_NUMBER: %w", err)
  }
 
+ var headCommit string
+ headCommit, err = git.HeadCommit(git.RunGit)
+ if err != nil {
+ return errors.New("failed to get the HEAD commit")
+ }
+
  timeout, _ := time.ParseDuration(meta.cfg.Repository.GitHub.Timeout)
  var gr reporter.GithubReporter
  if gr, err = reporter.NewGithubReporter(
@@ -203,11 +209,11 @@ func actionCI(c *cli.Context) error {
  meta.cfg.Repository.GitHub.Repo,
  prNum,
  meta.cfg.Repository.GitHub.MaxComments,
- git.RunGit,
+ headCommit,
  ); err != nil {
  return err
  }
- reps = append(reps, gr)
+ reps = append(reps, reporter.NewCommentReporter(gr))
  }
 
  minSeverity, err := checks.ParseSeverity(c.String(failOnFlag))

docs/changelog.md

@@ -5,6 +5,7 @@
 ### Changed
 
 - [promql/vector_matching](checks/promql/vector_matching.md) will now report more details, including which Prometheus server reports problems and which part of the query is the issue.
+- GitHub report code was refactored, it should behave as before.
 
 ## v0.62.2
 

internal/reporter/comments.go

@@ -2,20 +2,22 @@ package reporter
 
 import (
  "context"
+ "log/slog"
  "slices"
  "strings"
 
  "github.com/cloudflare/pint/internal/checks"
+ "github.com/cloudflare/pint/internal/output"
 )
 
-type PendingCommentV2 struct {
+type PendingComment struct {
  path string
  text string
  line int
  anchor checks.Anchor
 }
 
-type ExistingCommentV2 struct {
+type ExistingComment struct {
  meta any
  path string
  text string
@@ -26,14 +28,27 @@ type Commenter interface {
  Describe() string
  Destinations(context.Context) ([]any, error)
  Summary(context.Context, any, Summary, []error) error
- List(context.Context, any) ([]ExistingCommentV2, error)
- Create(context.Context, any, PendingCommentV2) error
- Delete(context.Context, any, ExistingCommentV2) error
- CanCreate(int) (bool, error)
- IsEqual(ExistingCommentV2, PendingCommentV2) bool
+ List(context.Context, any) ([]ExistingComment, error)
+ Create(context.Context, any, PendingComment) error
+ Delete(context.Context, any, ExistingComment) error
+ CanCreate(int) bool
+ CanDelete(ExistingComment) bool
+ IsEqual(ExistingComment, PendingComment) bool
 }
 
-func makeComments(summary Summary) (comments []PendingCommentV2) {
+func NewCommentReporter(c Commenter) CommentReporter {
+ return CommentReporter{c: c}
+}
+
+type CommentReporter struct {
+ c Commenter
+}
+
+func (cr CommentReporter) Submit(summary Summary) (err error) {
+ return Submit(context.Background(), summary, cr.c)
+}
+
+func makeComments(summary Summary) (comments []PendingComment) {
  var buf strings.Builder
  for _, reports := range dedupReports(summary.reports) {
  mergeDetails := identicalDetails(reports)
@@ -79,7 +94,7 @@ func makeComments(summary Summary) (comments []PendingCommentV2) {
  }
  }
 
- comments = append(comments, PendingCommentV2{
+ comments = append(comments, PendingComment{
  anchor: reports[0].Problem.Anchor,
  path: reports[0].Path.SymlinkTarget,
  line: line,
@@ -155,3 +170,126 @@ func problemIcon(s checks.Severity) string {
  return ":stop_sign:"
  }
 }
+
+func errsToComment(errs []error) string {
+ var buf strings.Builder
+ buf.WriteString("There were some errors when pint was trying to create a report.\n")
+ buf.WriteString("Some review comments might be outdated or missing.\n")
+ buf.WriteString("List of all errors:\n\n")
+ for _, err := range errs {
+ buf.WriteString("- `")
+ buf.WriteString(err.Error())
+ buf.WriteString("`\n")
+ }
+ return buf.String()
+}
+
+func Submit(ctx context.Context, s Summary, c Commenter) error {
+ slog.Info("Will now report problems", slog.String("reporter", c.Describe()))
+ dsts, err := c.Destinations(ctx)
+ if err != nil {
+ return err
+ }
+
+ for _, dst := range dsts {
+ slog.Info("Found a report destination", slog.String("reporter", c.Describe()), slog.Any("dst", dst))
+ if err = updateDestination(ctx, s, c, dst); err != nil {
+ return err
+ }
+ }
+
+ slog.Info("Finished reporting problems", slog.String("reporter", c.Describe()))
+ return nil
+}
+
+func updateDestination(ctx context.Context, s Summary, c Commenter, dst any) (err error) {
+ slog.Info("Listing existing comments", slog.String("reporter", c.Describe()))
+ existingComments, err := c.List(ctx, dst)
+ if err != nil {
+ return err
+ }
+
+ var created int
+ var errs []error
+ pendingComments := makeComments(s)
+ for _, pending := range pendingComments {
+ slog.Debug("Got pending comment",
+ slog.String("reporter", c.Describe()),
+ slog.String("path", pending.path),
+ slog.Int("line", pending.line),
+ slog.String("msg", pending.text),
+ )
+ for _, existing := range existingComments {
+ if c.IsEqual(existing, pending) {
+ slog.Debug("Comment already exists",
+ slog.String("reporter", c.Describe()),
+ slog.String("path", pending.path),
+ slog.Int("line", pending.line),
+ )
+ goto NEXTCreate
+ }
+ }
+ slog.Debug("Comment doesn't exist yet and needs to be created",
+ slog.String("reporter", c.Describe()),
+ slog.String("path", pending.path),
+ slog.Int("line", pending.line),
+ )
+
+ if !c.CanCreate(created) {
+ slog.Debug("Cannot create new comment",
+ slog.String("reporter", c.Describe()),
+ slog.String("path", pending.path),
+ slog.Int("line", pending.line),
+ )
+ goto NEXTCreate
+ }
+
+ if err := c.Create(ctx, dst, pending); err != nil {
+ slog.Error("Failed to create a new comment",
+ slog.String("reporter", c.Describe()),
+ slog.String("path", pending.path),
+ slog.Int("line", pending.line),
+ slog.Any("err", err),
+ )
+ return err
+ }
+ created++
+ NEXTCreate:
+ }
+
+ for _, existing := range existingComments {
+ for _, pending := range pendingComments {
+ if c.IsEqual(existing, pending) {
+ goto NEXTDelete
+ }
+ }
+ if !c.CanDelete(existing) {
+ goto NEXTDelete
+ }
+ if err := c.Delete(ctx, dst, existing); err != nil {
+ slog.Error("Failed to delete a stale comment",
+ slog.String("reporter", c.Describe()),
+ slog.String("path", existing.path),
+ slog.Int("line", existing.line),
+ slog.Any("err", err),
+ )
+ errs = append(errs, err)
+ }
+ NEXTDelete:
+ }
+
+ slog.Info("Creating report summary",
+ slog.String("reporter", c.Describe()),
+ slog.Int("reports", len(s.reports)),
+ slog.Int("online", int(s.OnlineChecks)),
+ slog.Int("offline", int(s.OnlineChecks)),
+ slog.String("duration", output.HumanizeDuration(s.Duration)),
+ slog.Int("entries", s.TotalEntries),
+ slog.Int("checked", int(s.CheckedEntries)),
+ )
+ if err := c.Summary(ctx, dst, s, errs); err != nil {
+ return err
+ }
+
+ return nil
+}