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

Split summary comment if exceeded max characters #647

Merged
merged 16 commits into from
Mar 24, 2024

Conversation

attiasas
Copy link
Contributor

@attiasas attiasas commented Feb 22, 2024

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.

On BitBucket, they limit the maximum number of characters per comment.
In case we have a PR that has a lot of issues, this results in an error when trying to create the comment:
11:07:00 [Error] couldn't add pull request comment: Status: 400 Bad Request, Body: {"errors":[{"context":"text","message":"Please enter a non-empty value less than 32768 characters","exceptionName":null}]}

For Azure, the limit for description size is 4000 chars, if exceeded it creates the following error:
image

Changes:

  • Mark Summary and Review comments the same (stop detecting by banner anymore).
  • If the maximum number of characters is exceeded for the summary comment, split it into multiple comments

@attiasas attiasas added bug Something isn't working safe to test Approve running integration tests on a pull request labels Feb 22, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Feb 22, 2024
@attiasas attiasas marked this pull request as draft February 22, 2024 09:13
@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Feb 29, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Feb 29, 2024
@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Feb 29, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Feb 29, 2024
@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Feb 29, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Feb 29, 2024
@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Mar 3, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Mar 3, 2024
@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Mar 3, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Mar 3, 2024
@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Mar 3, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Mar 3, 2024
Copy link
Contributor

@eranturgeman eranturgeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. did you verify that the new added line + split comment length doesn't pass the max possible chars in a comment?

  2. what happens if the split it exactly in one of the titles? the first part of the title will get the ## or ### and will be presented in a big and bold font, but the second part of the title will not. will it present it as we want?

utils/comment.go Outdated Show resolved Hide resolved
@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Mar 24, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Mar 24, 2024
@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Mar 24, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Mar 24, 2024
Copy link
Contributor

👍 Frogbot scanned this pull request and found that it did not add vulnerable dependencies.


@attiasas attiasas merged commit 206b4b8 into jfrog:dev Mar 24, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants