Code review tool git-appraise intergration #66

@bkcsoft commented on GitHub (Dec 26, 2016):

It just feels like it will break very fast. How does it handle "merge-conflicts" etc?

@bkcsoft commented on GitHub (Dec 26, 2016): It just feels like it will break very fast. How does it handle "merge-conflicts" etc?

GiteaMirror commented

@ojarjur commented on GitHub (Jan 3, 2017):

@bkcsoft The metadata formats used by git-appraise are designed to work with the git-notes 'cat_sort_uniq' merge strategy, so merge conflicts are not an issue (they are automatically resolved by the git notes merge command).

@ojarjur commented on GitHub (Jan 3, 2017): @bkcsoft The metadata formats used by git-appraise are designed to work with the git-notes ['cat_sort_uniq'](https://git-scm.com/docs/git-notes#_notes_merge_strategies) merge strategy, so merge conflicts are not an issue (they are automatically resolved by the `git notes merge` command).

GiteaMirror commented

@ojarjur commented on GitHub (Jan 4, 2017):

@bkcsoft I realized a little earlier that what you were worried about with merge conflicts might have been "what happens when a comment is edited?", and that deserves a bit more details:

The formats git-appraise uses do not represent the current state of a review, but rather a log of events (e.g. "User A added a comment at line B").

This means we can automatically resolve conflicts because we can take the union of all events (this is what the 'cat_sort_uniq' strategy gives us).

However, there is a gotcha in that the current set of events does not include an 'edit comment' event (it does support 'edit review request' events).

I've filed this issue to add such support.

@ojarjur commented on GitHub (Jan 4, 2017): @bkcsoft I realized a little earlier that what you were worried about with merge conflicts might have been "what happens when a comment is edited?", and that deserves a bit more details: The formats git-appraise uses do not represent the current state of a review, but rather a log of events (e.g. "User A added a comment at line B"). This means we can automatically resolve conflicts because we can take the union of all events (this is what the 'cat_sort_uniq' strategy gives us). However, there is a gotcha in that the current set of events does not include an 'edit comment' event (it does support 'edit review request' events). I've filed [this issue](https://github.com/google/git-appraise/issues/67) to add such support.

GiteaMirror commented

@stevenroose commented on GitHub (Jan 4, 2017):

@ojarjur Wow, it's a real honor to have you here as the original git-appraise author! Hopefully collaboration will allow this issue to move forward more quickly!

@stevenroose commented on GitHub (Jan 4, 2017): @ojarjur Wow, it's a real honor to have you here as the original git-appraise author! Hopefully collaboration will allow this issue to move forward more quickly!

GiteaMirror commented

@choucavalier commented on GitHub (Jan 22, 2017):

What is the current status of this?

@choucavalier commented on GitHub (Jan 22, 2017): What is the current status of this?

GiteaMirror commented

@lunny commented on GitHub (Jan 22, 2017):

Waiting for someone send PR. :)

@lunny commented on GitHub (Jan 22, 2017): Waiting for someone send PR. :)

GiteaMirror commented

@vizcay commented on GitHub (Sep 6, 2017):

There is someone currently working on this? There is https://github.com/google/git-appraise-web to use as an starting point..

@vizcay commented on GitHub (Sep 6, 2017): There is someone currently working on this? There is https://github.com/google/git-appraise-web to use as an starting point..

GiteaMirror commented

@strk commented on GitHub (Sep 6, 2017):

See #733 for an ongoing effort (stuck, as far as I can tell)

@strk commented on GitHub (Sep 6, 2017): See #733 for an ongoing effort (stuck, as far as I can tell)

GiteaMirror commented

@vizcay commented on GitHub (Sep 12, 2017):

It will be nice until #733 gets developed to be able to reference sections of code in the discussion of the pull request. This should be very easy to implement:

Add a small "copy link" in the ruler to the left of each file to reference that exact position, to be able to paste it in the comments.
Via js when the user clicks that link it navigates to that section of the code (even highlight it maybe).

That way it will be possible to discuss sections of code more easily, that's what people is interested in general I believe.

@vizcay commented on GitHub (Sep 12, 2017): It will be nice until #733 gets developed to be able to reference sections of code in the discussion of the pull request. This should be very easy to implement: * Add a small "copy link" in the ruler to the left of each file to reference that exact position, to be able to paste it in the comments. * Via js when the user clicks that link it navigates to that section of the code (even highlight it maybe). That way it will be possible to discuss sections of code more easily, that's what people is interested in general I believe.

GiteaMirror commented

@OmarAssadi commented on GitHub (Oct 19, 2017):

Would definitely be nice to see some sort of code review in Gitea. Added onto the original bounty. By the way, your bountysource badge appears to be broken!

Here is the current total:

@OmarAssadi commented on GitHub (Oct 19, 2017): Would definitely be nice to see some sort of code review in Gitea. Added onto the original bounty. By the way, your bountysource badge appears to be broken! Here is the current total: [![current amount](https://api.bountysource.com/badge/issue?issue_id=39308681)](https://www.bountysource.com/issues/39308681-code-review-tool-git-appraise-intergration)

GiteaMirror commented

@ypnos commented on GitHub (Jan 5, 2018):

The bounty is now over $200. Would be great if somebody finds the time to get this killer feature (code review) done. A good start for Gitea into 2018!

@ypnos commented on GitHub (Jan 5, 2018): The bounty is now over $200. Would be great if somebody finds the time to get this killer feature (code review) done. A good start for Gitea into 2018!

GiteaMirror commented

@hickscorp commented on GitHub (Mar 15, 2018):

Hey - picking up the discussion on this very important feature, it's good to see that there is some activity.
I see that the bounty is right now $210, what do you guys think it should reach to potentially be fair to pay whoever will be contributing to this (Trying to get a feel of the general consensus here not of a price tag)?

@lunny BTW thanks for taking a stab at it, even if it didn't pan out like something usable. At least you've shown everyone else that the comments should be per-pull-request, not global.

Also, @ojarjur being around this discussion feed, I'm pretty sure he'd give more advice about git-appraise if needed.

Also, and given the status of #733, I think a specification should be established before the work order is issued - as that last PR got stuck because inline comments were not per-pull-request.

As @vizcay suggests, there are two things that could be useful here:

1. The actual pull-request review flow, with inline comments, that would involve backend and database work,
1. The ability to easily manipulate line references in comments in general (including other parts of the app), using a more client-side href / anchor thing.

I also agree with @vizcay in the sense that the 2nd point is a very good way to mitigate the lack of the 1st point, as what the users are aiming for is a way to collaborate and talk about code-points in particular - and I can definitively see myself following a discussion on the main PR page while Ctrl+Clicking line numbers.

Therefore, another and 3rd way of mitigating both things would be an in-between solution. How about:

Whenever someone references a line of code in a comment, then the backend keeps track of those in a dedicated table, and potentially computes the composite data required for fast indexing and later query "per-file",
Then when the backend generates the output for a file, it would place a bullet on the left margin if that file + line was referenced in a comment, allowing to click it and jump to that (those) particular comment(s).

This way, the PR main screen would still syndicate discussion, while file views would stick to do what they do - viewing files.

I guess that later approach could be also an innovative way of doing what users are asking for, without all the burden of the complete task, while maybe differentiating Gitea from the others ;)

EDIT: BTW I absolutely know how frustrating it is as a maintainer to have peeps +1 a feature request all the time - while they could be doing it themselves. That is not the intent of this comment, I'm indeed available for helping more in the thinking process. I gave up on Golang years ago tho, so I won't produce any code at all as I'm way too rusty (Or should I say elixiry). I could also tip-in, if that's ultimately what's missing.

@hickscorp commented on GitHub (Mar 15, 2018): Hey - picking up the discussion on this very important feature, it's good to see that there is some activity. I see that the bounty is right now $210, what do you guys think it should reach to potentially be fair to pay whoever will be contributing to this (Trying to get a feel of the general consensus here not of a price tag)? @lunny BTW thanks for taking a stab at it, even if it didn't pan out like something usable. At least you've shown everyone else that the comments should be per-pull-request, not global. Also, @ojarjur being around this discussion feed, I'm pretty sure he'd give more advice about `git-appraise` if needed. Also, and given the status of #733, I think a specification should be established before the work order is issued - as that last PR got stuck because inline comments were not per-pull-request. As @vizcay suggests, there are two things that could be useful here: - 1. The actual pull-request review flow, with inline comments, that would involve backend and database work, - 2. The ability to easily manipulate line references in comments in general (including other parts of the app), using a more client-side href / anchor thing. I also agree with @vizcay in the sense that the 2nd point is a very good way to mitigate the lack of the 1st point, as what the users are aiming for is a way to collaborate and talk about code-points in particular - and I can definitively see myself following a discussion on the main PR page while Ctrl+Clicking line numbers. Therefore, another and 3rd way of mitigating both things would be an in-between solution. How about: - Whenever someone references a line of code in a comment, then the backend keeps track of those in a dedicated table, and potentially computes the composite data required for fast indexing and later query "per-file", - Then when the backend generates the output for a file, it would place a bullet on the left margin if that file + line was referenced in a comment, allowing to click it and jump to that (those) particular comment(s). This way, the PR main screen would still syndicate discussion, while file views would stick to do what they do - viewing files. I guess that later approach could be also an innovative way of doing what users are asking for, without all the burden of the complete task, while maybe differentiating Gitea from the others ;) EDIT: BTW I absolutely know how frustrating it is as a maintainer to have peeps +1 a feature request all the time - while they could be doing it themselves. That is not the intent of this comment, I'm indeed available for helping more in the thinking process. I gave up on *Golang* years ago tho, so I won't produce any code at all as I'm way too rusty (Or should I say elixiry). I could also tip-in, if that's ultimately what's missing.

GiteaMirror commented