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

allow comments on non-diff lines (#32257) #32259

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

abhishek818
Copy link

@abhishek818 abhishek818 commented Oct 14, 2024

closes #32257
/claim #32257

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 14, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 14, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Oct 14, 2024
@abhishek818 abhishek818 force-pushed the comment_on_non_diff_lines branch from 350613c to a65f0f6 Compare October 14, 2024 18:40
@lunny
Copy link
Member

lunny commented Oct 14, 2024

Once a user comments in the non-diff lines, it should be displayed in the changed files. So we need to automatically expand those lines that have non-diff comments. Or we can have a indicate icon there are comments hidden in non-diff code lines.

@yp05327
Copy link
Member

yp05327 commented Oct 15, 2024

IIRC, GitHub can only comment on those no displayed lines at first. I think this is for the concern mentioned above. So simply change the condition is not right I think.
e.g.
image
image

@abhishek818 abhishek818 marked this pull request as draft October 18, 2024 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 Bounty claim lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/go Pull requests that update Go code size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Commenting unchanged lines
4 participants