-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
feat: add inline comments on commits #36398
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?
feat: add inline comments on commits #36398
Conversation
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
This pull request adds inline comment functionality on commits, enabling users to add comments directly on commit diff lines similar to how comments work in pull request file reviews. This addresses issue #4898.
Changes:
- Added new database models for commit comments and reactions with CRUD operations
- Implemented handler routes for creating, updating, deleting commit comments and managing reactions
- Extended frontend JavaScript to handle comment interactions on commit diffs using event delegation
- Updated templates to conditionally render commit comment forms and context menus based on commit context
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| models/git/commit_comment.go | New model for storing commit-level code comments |
| models/git/commit_comment_reaction.go | New model for reactions on commit comments |
| models/git/commit_comment_test.go | Unit tests for commit comment model operations |
| routers/web/repo/commit_comment.go | HTTP handlers for commit comment CRUD and reactions |
| routers/web/repo/commit.go | Integration of commit comments into commit diff view |
| routers/web/web.go | New routes for commit comment endpoints |
| services/gitdiff/gitdiff.go | Service layer for loading commit comments into diff structures |
| web_src/js/features/repo-legacy.ts | Initialize comment handlers globally for commit pages |
| web_src/js/features/repo-issue.ts | Updated event delegation to pointerdown for DOM stability |
| web_src/js/features/repo-issue-edit.ts | Enhanced edit/quote handlers with data-target support for detached menus |
| web_src/js/features/repo-diff.ts | Updated form submission to handle commit comment responses |
| templates/repo/issue/view_content/context_menu.tmpl | Context menu with commit-specific URLs and permissions |
| templates/repo/diff/section_unified.tmpl | Show add-comment buttons on commit diffs with permission checks |
| templates/repo/diff/section_split.tmpl | Show add-comment buttons on commit diffs with permission checks |
| templates/repo/diff/comments.tmpl | Render comments with commit-specific reaction and edit URLs |
| templates/repo/diff/comment_form.tmpl | Form action routing for commit vs PR contexts |
| templates/repo/diff/box.tmpl | New comment URL configuration for commit diffs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0fe10b6 to
47a3bc9
Compare
Co-authored-by: Copilot <[email protected]> Signed-off-by: Jayant Pranjal <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Jayant Pranjal <[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
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@lunny can you check out this PR?? |
| initRepoIssueCommentDelete(); | ||
| initRepoIssueCodeCommentCancel(); | ||
| initRepoIssueReferenceIssue(); | ||
| initCompReactionSelector(); |
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.
These 5 are all for the comment editor, right? If yes, put them into a new initRepoCodeComments and use it also to initialize the other comments editors.
|
Please answer the concerns in #4898 first |
|
Hi, from my understanding the use of commit comments would be mostly for having file specific information only for specific commits. |
|
Also, I am looking at issue: #24635. Can you please confirm if it is available to work upon as in if it is something that is planned to be brought in. I would be happy to work on it, and also if you could assign to me the issue, that would be great. |
Which one is your case? By either, you need to be familiar with the code base, fully understand the details and have a complete design. |
|
I actually came here looking for bounty, but I have been getting familiar with the code with quite some time. I actually left a comment (#4898 (comment)) quite some time before for this |
So, why this feature is needed as of today? How end users would really use it? |
|
From what I understand we have messages for commits when we push the code, but to also have some inline comments pointing to specific line, we can have this feature. Ig, the people who need this feature might have some better ideas, but lets say someone has something to say about some commit which has already been pushed as in reviews to some already existing or merged pr, this might be helpful. Or lets say someone has a repository with some educational content, they can point to specific lines with comments indicating use of specific parts of code |
Why not open an issue or a PR? If the repo gets new commits, how to find these "ideas" again?
Why not directly comment in the related PR?
Why not write the "educational content" into code comments? If the author pushes some new commits, how to find the old comments? |
|
Yeahh, I agree there are ways to do this over implementing a completely new feature. |
|
To be completely honest I mostly came here looking for bounties |
fixes #4898
/claim #4898