Skip to content

Adding support for issue-body with more than 65536 words, issue body to be created as an issue comment #167

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

Merged
merged 25 commits into from
Apr 26, 2025

Conversation

snskArora
Copy link
Collaborator

This has PR 2 major changes:

Note

Please perform a squash merge so as to not hinder ease of commit tracking as there are quite a few incoming commits.

@snskArora snskArora marked this pull request as ready for review March 22, 2025 14:09
@snskArora
Copy link
Collaborator Author

This would resolve #89

@snskArora snskArora added this to the 1.10 milestone Mar 22, 2025
@snskArora snskArora removed this from the 1.10 milestone Mar 30, 2025
@snskArora
Copy link
Collaborator Author

@lizziemac @beverts312, please review this PR. It would be a nice feature to have within the new release.

@lizziemac
Copy link
Collaborator

@snskArora this looks good, and it works, however there are couple issues and questions:

  • the reported max is characters, not words
  • some users can see character limits of 200k+, depending on the zipped size of the issue/comment https://github.com/orgs/community/discussions/41331?utm_source=chatgpt.com#discussioncomment-4366258
  • we should limit to characters, not words, and have that limit be 65536, even though it seems sometimes (even in your real-life test) it works beyond that, to ensure this is supported for everyone
  • was the issue body supposed to be in the issue message, then split from there? or are we just automatically making it a comment if it's detected to be too big? your example you provide only has the message in the comments.

@snskArora
Copy link
Collaborator Author

  • The looping is done over words, but the limit threshold is in characters only. I am processing over words to avoid breaking a word between two comments. The only bottleneck it creates is that if a word is more than 65536 characters, it will fail.

  • Earlier, it was in a single comment, but now, since we are going to have an issue comment, I feel it would be better to have the approval body as a separate block than instructions on working with the approval, for better clarity.

@lizziemac
Copy link
Collaborator

The looping is done over words, but the limit threshold is in characters only. I am processing over words to avoid breaking a word between two comments. The only bottleneck it creates is that if a word is more than 65536 characters, it will fail.

ah my bad, I think I was a little quick when reviewing. please update the PR name though

Earlier, it was in a single comment, but now, since we are going to have an issue comment, I feel it would be better to have the approval body as a separate block than instructions on working with the approval, for better clarity.

noted. I think that's fine

lizziemac
lizziemac previously approved these changes Apr 24, 2025
Copy link
Collaborator

@lizziemac lizziemac left a comment

Choose a reason for hiding this comment

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

good to merge, just update the PR name and make 65536 a constant/var

@snskArora snskArora changed the title Adding support for issue-body with more than 65536 words Adding support for issue-body with more than 65536 words, issue body to be created as an issue message Apr 26, 2025
@snskArora snskArora changed the title Adding support for issue-body with more than 65536 words, issue body to be created as an issue message Adding support for issue-body with more than 65536 words, issue body to be created as an issue comment Apr 26, 2025
@snskArora snskArora merged commit bfb0ba3 into trstringer:main Apr 26, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants