-
Notifications
You must be signed in to change notification settings - Fork 564
WIP: support single comment summary with orchestrator mode #1850
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
base: develop
Are you sure you want to change the base?
Changes from 1 commit
e340b53
151068c
60a7a13
ffa6e55
dfe91e9
5a33821
eac4651
6df110b
44c1aff
f552996
b4068cd
1a48f98
1d3bc41
4c9fb5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -107,6 +107,42 @@ type ReportStrategy interface { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Report(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, supportsCollapsibleComment bool) (commentId string, commentUrl string, error error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
type AlwaysSameCommentStrategy struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Title string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
CommentId string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
TimeOfRun time.Time | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (strategy AlwaysSameCommentStrategy) Report(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, supportsCollapsibleComment bool) (string, string, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
comments, err := ciService.GetComments(PrNumber) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return "", "", fmt.Errorf("error getting comments: %v", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var commentBody *string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var commentUrl string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for _, comment := range comments { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if comment.Id == strategy.CommentId { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
commentBody = comment.Body | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
commentUrl = comment.Url | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var reportTitle string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if strategy.Title != "" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
reportTitle = strategy.Title + " " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
reportTitle = "Digger run report at " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
err = appendToExistingComment(ciService, PrNumber, *commentBody, report, reportTitle, strategy.CommentId, supportsCollapsibleComment) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return "", "", fmt.Errorf("error while adding to existing comment: %v", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return strategy.CommentId, commentUrl, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
func (strategy AlwaysSameCommentStrategy) Report(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, supportsCollapsibleComment bool) (string, string, error) { | |
comments, err := ciService.GetComments(PrNumber) | |
if err != nil { | |
return "", "", fmt.Errorf("error getting comments: %v", err) | |
} | |
var commentBody *string | |
var commentUrl string | |
for _, comment := range comments { | |
if comment.Id == strategy.CommentId { | |
commentBody = comment.Body | |
commentUrl = comment.Url | |
} | |
} | |
var reportTitle string | |
if strategy.Title != "" { | |
reportTitle = strategy.Title + " " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)") | |
} else { | |
reportTitle = "Digger run report at " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)") | |
} | |
err = appendToExistingComment(ciService, PrNumber, *commentBody, report, reportTitle, strategy.CommentId, supportsCollapsibleComment) | |
if err != nil { | |
return "", "", fmt.Errorf("error while adding to existing comment: %v", err) | |
} | |
return strategy.CommentId, commentUrl, err | |
} | |
func (strategy AlwaysSameCommentStrategy) Report(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, supportsCollapsibleComment bool) (string, string, error) { | |
comments, err := ciService.GetComments(PrNumber) | |
if err != nil { | |
return "", "", fmt.Errorf("error getting comments: %v", err) | |
} | |
var commentBody *string | |
var commentUrl string | |
for _, comment := range comments { | |
if comment.Id == strategy.CommentId { | |
commentBody = comment.Body | |
commentUrl = comment.Url | |
} | |
} | |
var reportTitle string | |
if strategy.Title != "" { | |
reportTitle = strategy.Title + " " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)") | |
} else { | |
reportTitle = "Digger run report at " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)") | |
} | |
if commentBody == nil { | |
return "", "", fmt.Errorf("no comment found with CommentId: %v", strategy.CommentId) | |
} | |
err = appendToExistingComment(ciService, PrNumber, *commentBody, report, reportTitle, strategy.CommentId, supportsCollapsibleComment) | |
if err != nil { | |
return "", "", fmt.Errorf("error while adding to existing comment: %v", err) | |
} | |
return strategy.CommentId, commentUrl, err | |
} |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent slice bounds out of range in appendToExistingComment
If existingCommentBody
has fewer than two lines, slicing lines[1:len(lines)-1]
will cause a panic. Ensure that lines
has at least two elements before slicing.
// strip first and last lines
lines := strings.Split(existingCommentBody, "\n")
+ if len(lines) >= 2 {
lines = lines[1 : len(lines)-1]
+ } else {
+ lines = []string{}
+ }
existingCommentBody = strings.Join(lines, "\n")
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
func appendToExistingComment(ciService ci.PullRequestService, prNumber int, existingCommentBody string, reportToAppend string, reportTitle string, commentId string, supportsCollapsible bool) error { | |
// strip first and last lines | |
lines := strings.Split(commentBody, "\n") | |
lines := strings.Split(existingCommentBody, "\n") | |
lines = lines[1 : len(lines)-1] | |
commentBody = strings.Join(lines, "\n") | |
existingCommentBody = strings.Join(lines, "\n") | |
commentBody = commentBody + "\n\n" + report + "\n" | |
existingCommentBody = existingCommentBody + "\n\n" + reportToAppend + "\n" | |
var completeComment string | |
if !supportsCollapsible { | |
completeComment = utils.AsComment(reportTitle)(commentBody) | |
completeComment = utils.AsComment(reportTitle)(existingCommentBody) | |
} else { | |
completeComment = utils.AsCollapsibleComment(reportTitle, false)(commentBody) | |
completeComment = utils.AsCollapsibleComment(reportTitle, false)(existingCommentBody) | |
} | |
err := ciService.EditComment(PrNumber, commentIdForThisRun, completeComment) | |
err := ciService.EditComment(prNumber, commentId, completeComment) | |
if err != nil { | |
return "", "", fmt.Errorf("error editing comment: %v", err) | |
return fmt.Errorf("error editing comment: %v", err) | |
} | |
return fmt.Sprintf("%v", commentIdForThisRun), commentUrl, nil | |
return nil | |
func appendToExistingComment(ciService ci.PullRequestService, prNumber int, existingCommentBody string, reportToAppend string, reportTitle string, commentId string, supportsCollapsible bool) error { | |
// strip first and last lines | |
lines := strings.Split(existingCommentBody, "\n") | |
if len(lines) >= 2 { | |
lines = lines[1 : len(lines)-1] | |
} else { | |
lines = []string{} | |
} | |
existingCommentBody = strings.Join(lines, "\n") | |
existingCommentBody = existingCommentBody + "\n\n" + reportToAppend + "\n" | |
var completeComment string | |
if !supportsCollapsible { | |
completeComment = utils.AsComment(reportTitle)(existingCommentBody) | |
} else { | |
completeComment = utils.AsCollapsibleComment(reportTitle, false)(existingCommentBody) | |
} | |
err := ciService.EditComment(prNumber, commentId, completeComment) | |
if err != nil { | |
return fmt.Errorf("error editing comment: %v", err) | |
} | |
return nil |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -125,6 +125,13 @@ func (r ReporterProvider) GetReporter(title string, reporterSpec ReporterSpec, c | |||||||||||||||||||||||||||||||
return reporting.LatestRunCommentStrategy{ | ||||||||||||||||||||||||||||||||
TimeOfRun: time.Now(), | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
case "always_same_comment": | ||||||||||||||||||||||||||||||||
return reporting.AlwaysSameCommentStrategy{ | ||||||||||||||||||||||||||||||||
Title: title, | ||||||||||||||||||||||||||||||||
TimeOfRun: time.Now(), | ||||||||||||||||||||||||||||||||
CommentId: *reporterSpec.ReportCommentId, | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
|
case "always_same_comment": | |
return reporting.AlwaysSameCommentStrategy{ | |
Title: title, | |
TimeOfRun: time.Now(), | |
CommentId: *reporterSpec.ReportCommentId, | |
} | |
case "always_same_comment": | |
if reporterSpec.ReportCommentId == nil { | |
return reporting.MultipleCommentsStrategy{} | |
} | |
return reporting.AlwaysSameCommentStrategy{ | |
Title: title, | |
TimeOfRun: time.Now(), | |
CommentId: *reporterSpec.ReportCommentId, | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix redeclaration of batchId variable
The batchId variable is redeclared using := operator when it's already defined. This will cause a compilation error.
Also applies to: 958-958
🧰 Tools
🪛 golangci-lint (1.62.2)
538-538: no new variables on left side of :=
(typecheck)