Skip to content

Commit

Permalink
Refactor github reporer to use common comments code
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Jul 23, 2024
1 parent 615fa79 commit 4bb0c3e
Show file tree
Hide file tree
Showing 9 changed files with 782 additions and 567 deletions.
12 changes: 9 additions & 3 deletions cmd/pint/ci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(
Expand All @@ -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))
Expand Down
1 change: 1 addition & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
156 changes: 147 additions & 9 deletions internal/reporter/comments.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Loading

0 comments on commit 4bb0c3e

Please sign in to comment.