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

Add inline comments on commits #4898

Open
tonivj5 opened this issue Sep 9, 2018 · 55 comments
Open

Add inline comments on commits #4898

tonivj5 opened this issue Sep 9, 2018 · 55 comments
Labels
💎 Bounty issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/feature Completely new functionality. Can only be merged if feature freeze is not active.

Comments

@tonivj5
Copy link
Contributor

tonivj5 commented Sep 9, 2018

Add comments on commits (not only in PR), I would love to have this feature 😃

Reference: #124, #124 (comment)

Thanks to gitea's team and contributors ❤️

@jonasfranz jonasfranz added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Sep 9, 2018
@lunny lunny mentioned this issue Sep 11, 2018
2 tasks
@LukeOwlclaw
Copy link

Sorry, I don't want to be rude nor a smartass, I just want to understand. Is this issue about

Pull/Merge request inline comments

which is supposed to be already supported according to https://docs.gitea.io/en-us/comparison/ ?

image

@techknowlogick
Copy link
Member

@LukeOwlclaw this issue is inline comments on commits. Inline comments on code in PRs already (per the docs you linked). also please take this comment same way, I'm a bit sleepy due to jetlag so I apologize if this response doesn't come off as friendly (I tried to make it friendly).

@rakshith-ravi
Copy link
Contributor

@xxxTonixxx is there a PR that currently addresses this?

@tonivj5
Copy link
Contributor Author

tonivj5 commented Nov 22, 2018

I think no 😢

@rakshith-ravi
Copy link
Contributor

@techknowlogick I know open source projects hate asking for ETAs, and I really don't want to come off as rude, but I'm just curious how high on the priority list is this in? There doesn't seem to be a milestone listed, so what are the odds that we'll see this implemented in 1.7.x? Is this something the team is actively planning to work on or is it deferred for sometime later? Just curious so that I can set my expectations clear 😅

@techknowlogick
Copy link
Member

@rakshith-ravi sadly it won’t be in 1.7 as we are a in a feature freeze for that release, as for ETAs I cant say, as for the project we don’t have a roadmap and things are added to the project as PRs happen. That means even a non-team member could work on this, if you or someone you know has Holland skills. As a side note you don’t come off as rude and I really appreciate how you asked as it was really kind :)

@stale
Copy link

stale bot commented Feb 28, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Feb 28, 2019
@tonivj5
Copy link
Contributor Author

tonivj5 commented Feb 28, 2019

Not yet, please 😢

@stale stale bot removed the issue/stale label Feb 28, 2019
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Mar 1, 2019
@ghost
Copy link

ghost commented Mar 31, 2019

Yeah this would be fantastic!

@6543
Copy link
Member

6543 commented Jun 15, 2019

Vote

@lunny
Copy link
Member

lunny commented Jun 16, 2019

So If we can comment on any commit. We should consider some implementation things below:

  1. Where should the comments stored, currently all comments stored with an issue(pr)
  2. We have to notify all related users(who?) via UI/mail and etc.

@rakshith-ravi
Copy link
Contributor

I'd recommend a structure like this -
There would be two different kinds of comment on commits: One on the upstream code and one on the PR.

  • Both the comment on commit can be linked to the repo (the repo ID as the primary key). That way, Even when the PR gets merged, the comment still stays. We could use a composite key of the commit hash and the repo ID as the primary key.
  • As for the notification, we could perhaps notify all the users who are contributors of the repo. If it's a PR, I guess the PR owner will also be notified.

If I knew GoLang better, I would've implemented this myself. Unfortunately, I can only contribute in terms of software architecture

@zeripath
Copy link
Contributor

I suspect we should leverage git notes or git notes-like behaviour for this. I suspect that putting this in to the database is only going to cause trouble. I wonder if the comments on PRs should do the same.

@zeripath
Copy link
Contributor

@rakshith-ravi If you can program in an imperative language you can program in go. It's really not a very difficult language if you can get past its idiosyncrasies.

@lunny
Copy link
Member

lunny commented Jun 16, 2019

PR comments have been implemented. I think we are talking about commit comment without a PR.

@BaxAndrei
Copy link

BaxAndrei commented Nov 23, 2019

Yes, like on github
image

image

@0x416c69
Copy link

Vote.

Gitea really lacks this important feature. Everything else I could live on without but not this.

@guillep2k
Copy link
Member

guillep2k commented Nov 26, 2019

@0x416c69 Just so I understand your use case, why comments are so important in a specific commit? I've seen GitHub has that, but I never could picture a proper scenario for this. Commits get quickly lost in the sea of blobs....

@BaxAndrei
Copy link

I use this function. Especially to signal to the people who sent the commit that something is wrong, exempting me from opening an issue. It is also useful if I want to add something extra to that commit or mention something.

@lunny
Copy link
Member

lunny commented Nov 26, 2019

We also need refactor notification system to notify via ui/mail/webhook and etc.

@0x416c69
Copy link

@guillep2k Best way possible for me to talk about a certain commit is the commit comment. Although I could just stick with a messenger and a group talk, place the commit link/hash and talk about it there with my team but it will get really messy and the track of the conversation will be lost very quickly.

As a basic and common user of git, this I think is the most needed feature.

@jcfigueiredo
Copy link

@0x416c69 Just so I understand your use case, why comments are so important in a specific commit? I've seen GitHub has that, but I never could picture a proper scenario for this. Commits get quickly lost in the see of blobs....

We use it a lot for code reviewing PRs before merging to master.

@techknowlogick
Copy link
Member

@jcfigueiredo the review functionality already exists in PRs where you can comment on lines of code.

@Albirew
Copy link

Albirew commented Jan 8, 2020

this "may" also be useful to reference a commit or part of it to an issue without touching issue itself (examples: "may make issue #1 reappear", "this exact part breaks PR !2", etc.) or discussing about a specific part of code.

Again, these can already be done the other way around (eg:"commit b822518 may make this issue reappear", "the changes on modules/setting/queue.go lines 58-59 of commit c779ac1 breaks this PR", etc.)

@pinuke
Copy link

pinuke commented Nov 6, 2022

IMO, we're taking this GoFundMe thing too far. I get it. We all get it. Money doesn't grow on trees, but this is the open source community you guys are talking to. Money doesn't come from any of our pockets. This is the poor programmers club

GH issues is for contributors and users to work on features. Not electronically pan-handle each other.

Look I'm broke too, but this is just poorly representing the FOSS community (Emphasis on F OSS). The FOSS defacto standard for this is to just tell people "we're too broke to work on it right now," not "please give me money to do it"

@KaKi87

This comment was marked as spam.

@pinuke
Copy link

pinuke commented Nov 6, 2022

@KaKi87 That just makes it so much worse.

@pinuke
Copy link

pinuke commented Nov 6, 2022

Just so it's clear, these comments get sent out to subscribers through their emails. This means that this kind of behavior goes against the Github Acceptable Use Policy.

These kind of comments are also considerably off-topic which is also against multiple policies in the Acceptable Use Policies and GIthub Terms of Service. To find out what's on topic for GH Issues please see https://docs.github.com/en/issues/tracking-your-work-with-issues/about-issues

@go-gitea go-gitea locked and limited conversation to collaborators Nov 6, 2022
@lunny lunny modified the milestones: 1.19.0, 1.20.0 Jan 31, 2023
@delvh delvh removed this from the 1.20.0 milestone Jun 5, 2023
@go-gitea go-gitea unlocked this conversation Dec 19, 2023
@oktasense

This comment was marked as outdated.

@KaKi87
Copy link

KaKi87 commented Jan 13, 2024

In which version ?

@delvh
Copy link
Member

delvh commented Jan 13, 2024

Erm… What?
I think you have slightly misunderstood what is meant by that, @oktasense.
It is meant as
image
not what you have posted.

And that is not yet supported.

@egelhaus
Copy link

Is there any current Update to this Feature or?

Copy link

algora-pbc bot commented Oct 14, 2024

💎 $300 bounty • Gitea

Steps to solve:

  1. Start working: Comment /attempt #4898 with your implementation plan
  2. Submit work: Create a pull request including /claim #4898 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to go-gitea/gitea!

Add a bountyShare on socials

Attempt Started (GMT+0) Solution
🟢 @RedCocoon Oct 17, 2024, 8:49:13 AM #32384

@RedCocoon
Copy link

RedCocoon commented Oct 17, 2024

Personally really interested in this gitea thing so imma give it a shot
/attempt #4898

@RedCocoon
Copy link

RedCocoon commented Oct 21, 2024

I think I have come up with a solution, though not a good solution by any stretch of imagination.

First, the problem: Currently, conversations in Gitea are too closely tied to Issues to be reusable nicely anywhere else. Gitea's Pull Request itself from what I can see also janks it way by using Issues and that itself also causes a lot of troubles.

My proposed solution would be to refactor conversations so that they are standalone and can be reusable elsewhere. For my PR, I would only extract out conversations from Issues. I would however, not fix the jank introduced by Gitea's Pull Request conversation.

Please let me know if this is a good idea, or propose some other solutions. IMO my proposed solution is the best way moving forward. I've spent 4 days thinking and this is the best I could come up with. I really don't wanna do my proposed solution since it is a lot of work so if there is a better idea please suggest it!

I will now respond to every questions here first.

So If we can comment on any commit. We should consider some implementation things below:

1. Where should the comments stored, currently all comments stored with an issue(pr)

2. We have to notify all related users(who?) via UI/mail and etc.
  1. A new Conversation table, that is context-agnostic, such that everything that needs a conversation can just reuse it. (will add backwards compatibility for issues/pr.)
  2. Similar to Github's implementation, we will only notify all participants that have not opted out (a.k.a. everyone that commented). Different from IssuePR, there is no "First Poster" for a Conversation. Additionally, maybe the owner of the Repo and people watching the Repo? Feel free to discuss more regarding this.

I'd recommend a structure like this - There would be two different kinds of comment on commits: One on the upstream code and one on the PR.

* Both the comment on commit can be linked to the repo (the repo ID as the primary key). That way, Even when the PR gets merged, the comment still stays. We could use a composite key of the commit hash and the repo ID as the primary key.

A. Currently the comment are still there when a PR is merged, as PR is just using issues, so no problems there

I suspect we should leverage git notes or git notes-like behaviour for this. I suspect that putting this in to the database is only going to cause trouble. I wonder if the comments on PRs should do the same.

A. If my proposal is approved, someone else can take on the task of separating PRs from Issues and that is what I believe should be done eventually too.

We also need refactor notification system to notify via ui/mail/webhook and etc.

A. Yes. It will be a very massive pain.

@lunny
Copy link
Member

lunny commented Oct 21, 2024

Thinking the issue again, maybe a new table but not comment is better? Commit commments could be a standalone thing which will not mix with issues comment and pull request comments.

@RedCocoon
Copy link

The problem with that is we potentially need to re-implement quite a lot and maintain both Commit Comments and IssuePR Comments (which are more like ConversationEvent rather than Comments tbh).

I suppose I can make them separate for now, since I already decoupled everything related to comments from the front-end. Let me try to keep it separate for now.

For the future though someone should definitely unify them.

Copy link

algora-pbc bot commented Oct 30, 2024

💡 @RedCocoon submitted a pull request that claims the bounty. You can visit your bounty board to reward.

@anonhostpi
Copy link

One concern I have is that I think inline conversations should be linked to git blames. This way relevant conversations follow the correct positioning between commits and are automatically "hidden" when a line is overwritten.

I suggest that comments be linked with an web element system instead of being hard-tied to standard repository elements for flexibility. This would require a robust web element tracking system similar to SalesForce, but would allow you to attach conversations to anything.

@RedCocoon
Copy link

RedCocoon commented Oct 30, 2024 via email

@anonhostpi
Copy link

anonhostpi commented Oct 30, 2024

Another thought I just had is that attaching inline comments to blames would also allow conversations to be propagated between views (forks, code in issues, code in PRs, commits, older commits) and allow them to be seen in/added to other views like branch/head comparisons.

@RedCocoon
Copy link

RedCocoon commented Oct 30, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💎 Bounty issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

No branches or pull requests