-
Notifications
You must be signed in to change notification settings - Fork 29
feat: Split comments if too long #436
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: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Mmadu Manasseh <[email protected]>
Temporary image available at |
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
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.
Pull Request Overview
Adds support for splitting long VCS comments into multiple chunks with backoff retries and proper link headers.
- Extended the
Client
interface to include max comment length and PR link template, and updatedUpdateMessage
to accept multiple message parts. - Implemented retry logic using exponential backoff in both GitLab and GitHub clients.
- Added sophisticated comment-splitting logic in
BuildComment
to honor VCS size limits and preserve code blocks.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
pkg/vcs/types.go | Updated Client interface with new methods and changed UpdateMessage signature |
pkg/vcs/gitlab_client/message.go | Added backoff retry and multi-part UpdateMessage implementation |
pkg/vcs/github_client/message.go | Added backoff retry and multi-part UpdateMessage implementation |
pkg/vcs/github_client/backoff.go | Added backoff configuration helper |
pkg/msg/message.go | Enhanced BuildComment with splitting logic and code-block handling |
mocks/vcs/mocks/mock_MockClient.go | Updated mocks for new interface methods and UpdateMessage signature |
Comments suppressed due to low confidence (4)
pkg/vcs/types.go:21
- The
UpdateMessage
signature was changed to accept a PullRequest and a slice of strings, but the doc comment wasn’t updated. Please update the comment to reflect the new parameters.
// UpdateMessage update a message with new content
pkg/vcs/types.go:41
- The comment refers to
GetPrLinkTemplate
but the method is namedGetPrCommentLinkTemplate
. Update the comment for consistency.
// GetPrLinkTemplate returns the template for the PR link
pkg/msg/message.go:148
- [nitpick] The
BuildComment
function is quite large and handles many responsibilities (header/footer, splitting, code‐block preservation). Consider extracting the split logic into smaller helper functions to improve readability and testability.
// BuildComment iterates the map of all apps in this message, building a final comment from their current state
mocks/vcs/mocks/mock_MockClient.go:798
- [nitpick] The parameter name
strings
shadows the Gostrings
package and may be confusing. Consider renaming it tomessages
orcommentChunks
.
func (_mock *MockClient) UpdateMessage(context1 context.Context, pullRequest vcs.PullRequest, message *msg.Message, strings []string) error {
Signed-off-by: Mmadu Manasseh <[email protected]>
@MeNsaaH While it works (that's positive!), 2nd part of the comment (overflow) remains expanded and does not look good. I think this should be wrapped with
But I think this feature should cut at the application level (like this),
wdyt? |
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
the issue is with github, the max comment limit is 65553 characters which is far less than what's on gitlab (1million chars). With github there are cases, where the diff of single resource (object) could span multiple comments. So, splitting by object-level level would be impossible |
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Add feature to split comments across all vcs platforms while retaining codeblocks in between splits
Closes #421 #301