-
Notifications
You must be signed in to change notification settings - Fork 562
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
WIP: support single comment summary with orchestrator mode #1850
base: develop
Are you sure you want to change the base?
Changes from 5 commits
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 |
---|---|---|
|
@@ -499,7 +499,15 @@ func handlePullRequestEvent(gh utils.GithubClientProvider, payload *github.PullR | |
log.Printf("strconv.ParseInt error: %v", err) | ||
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not handle commentId: %v", err)) | ||
} | ||
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, models.DiggerVCSGithub, organisationId, impactedJobsMap, impactedProjectsMap, projectsGraph, installationId, branch, prNumber, repoOwner, repoName, repoFullName, commitSha, commentId, diggerYmlStr, 0) | ||
|
||
placeholderComment, err := ghService.PublishComment(prNumber, "digger report placehoder") | ||
if err != nil { | ||
log.Printf("strconv.ParseInt error: %v", err) | ||
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not create placeholder commentId for report: %v", err)) | ||
return fmt.Errorf("comment reporter error: %v", err) | ||
} | ||
|
||
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, models.DiggerVCSGithub, organisationId, impactedJobsMap, impactedProjectsMap, projectsGraph, installationId, branch, prNumber, repoOwner, repoName, repoFullName, commitSha, commentId, &placeholderComment.Id, diggerYmlStr, 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. -batchId, _, err := utils.ConvertJobsToDiggerJobs(
+batchId, _, err = utils.ConvertJobsToDiggerJobs( Also applies to: 958-958 🧰 Tools🪛 golangci-lint (1.62.2)538-538: no new variables on left side of := (typecheck)
Comment on lines
+525
to
+533
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Extract duplicate placeholder comment logic into a helper function The placeholder comment creation logic is duplicated between Extract the logic into a helper function: func createPlaceholderComment(ghService *dg_github.GithubService, number int, commentReporterManager utils.CommentReporterManager) (*github.IssueComment, error) {
placeholderComment, err := ghService.PublishComment(number, "digger report placeholder")
if err != nil {
log.Printf("failed to publish placeholder comment: %v", err)
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not create placeholder commentId for report: %v", err))
return nil, fmt.Errorf("comment reporter error: %v", err)
}
return placeholderComment, nil
} Also applies to: 942-949 |
||
if err != nil { | ||
log.Printf("ConvertJobsToDiggerJobs error: %v", err) | ||
commentReporterManager.UpdateComment(fmt.Sprintf(":x: ConvertJobsToDiggerJobs error: %v", err)) | ||
|
@@ -874,7 +882,14 @@ func handleIssueCommentEvent(gh utils.GithubClientProvider, payload *github.Issu | |
return fmt.Errorf("comment reporter error: %v", err) | ||
} | ||
|
||
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, "github", orgId, impactedProjectsJobMap, impactedProjectsMap, projectsGraph, installationId, *branch, issueNumber, repoOwner, repoName, repoFullName, *commitSha, reporterCommentId, diggerYmlStr, 0) | ||
placeholderComment, err := ghService.PublishComment(issueNumber, "digger report placehoder") | ||
if err != nil { | ||
log.Printf("strconv.ParseInt error: %v", err) | ||
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not create placeholder commentId for report: %v", err)) | ||
return fmt.Errorf("comment reporter error: %v", err) | ||
} | ||
Comment on lines
+942
to
+947
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Fix incorrect error message and consider refactoring duplicated code
First, fix the error message: placeholderComment, err := ghService.PublishComment(issueNumber, "digger report placehoder")
if err != nil {
- log.Printf("strconv.ParseInt error: %v", err)
+ log.Printf("failed to publish placeholder comment: %v", err)
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not create placeholder commentId for report: %v", err))
return fmt.Errorf("comment reporter error: %v", err)
} Consider extracting the placeholder comment creation into a shared helper function to avoid code duplication: func createPlaceholderComment(ghService *dg_github.GithubService, number int, commentReporterManager *utils.CommentReporterManager) (*github.IssueComment, error) {
placeholderComment, err := ghService.PublishComment(number, "digger report placeholder")
if err != nil {
log.Printf("failed to publish placeholder comment: %v", err)
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not create placeholder commentId for report: %v", err))
return nil, fmt.Errorf("comment reporter error: %v", err)
}
return placeholderComment, nil
} |
||
|
||
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, "github", orgId, impactedProjectsJobMap, impactedProjectsMap, projectsGraph, installationId, *branch, issueNumber, repoOwner, repoName, repoFullName, *commitSha, reporterCommentId, &placeholderComment.Id, diggerYmlStr, 0) | ||
if err != nil { | ||
log.Printf("ConvertJobsToDiggerJobs error: %v", err) | ||
commentReporterManager.UpdateComment(fmt.Sprintf(":x: ConvertJobsToDiggerJobs error: %v", err)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
-- Modify "digger_batches" table | ||
ALTER TABLE "public"."digger_batches" ADD COLUMN "placeholder_comment_id_for_report" text NULL; |
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 incorrect error message in placeholder comment creation
The error message incorrectly references "strconv.ParseInt" when the error is from publishing a comment.
Apply this fix:
📝 Committable suggestion