Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor github reporer to use common comments code #1037

Merged
merged 1 commit into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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