Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

Provide line by line commenting, review, conversation tools in file (blob) view #211

Open
matthewlmcclure opened this issue Jun 13, 2014 · 23 comments

Comments

@matthewlmcclure
Copy link

I'd like to leave a comment about the current state of the code on a given branch.

screen shot 2014-06-13 at 2 40 52 pm

It seems that as of 2014-06-13, to leave a comment on a given line in a given blob view, I have to navigate to Blame and then to the commit that introduced the line, where I can comment on the added line in the commit view.

screen shot 2014-06-13 at 2 41 09 pm

My comment will trigger notifications to repository watchers, to people I mention, and presumably to the commit's author, but the GitHub website doesn't surface the comment anywhere other than on that old commit. My guess is that it's unlikely that many people will find it.

screen shot 2014-06-13 at 2 41 53 pm

I could create an Issue instead, but issues connote a need for resolution, whereas I'm merely trying to create a greater opportunity for valuable action via starting a conversation.

Instead, I'd like to comment directly from the blob view to start a conversation about a given area of the code. I'd like those conversations to be easily retrievable via search and browsing.

I'm imagining something like Google Docs's commenting, where I can comment on a particular part of a document, a conversation can follow, and eventually one of the participants can Resolve the conversation.

Maybe "Conversations" should become Issues or have a similar but separate browsable and searchable index.

@matthewlmcclure matthewlmcclure changed the title Provide line by line commenting, review, conversation tools file (blob) view Provide line by line commenting, review, conversation tools in file (blob) view Jun 13, 2014
@cirosantilli
Copy link
Collaborator

IIUC this could only be doable from the raw view for files that have preview like Markdown.

@matthewlmcclure
Copy link
Author

Why do you think it wouldn't be doable for arbitrary files in the blob view?

@cirosantilli
Copy link
Collaborator

Would be non-trivial to associate comments to sources lines that show a special preview from the preview, like HTML preview of Markdown files on URLs of type blob/master as in your screenshot.

I'm interested in things like this, but the big question is: how do you transfer the comments if the file is changed, e.g. commented lines moved to another place? I don't think its computationally doable, only through heuristics.

Another possibility would be to show the comments only until the file is changed, but that could happen anytime for an unrelated reason, so it's not a very interesting possibility.

@matthewlmcclure
Copy link
Author

OK I understand what you're saying now. Yes I imagine that would be a challenge. Hopefully not an insurmountable one.

@captn3m0
Copy link

Showing the comments from the last commit only would be a easy solution. Plus, I think we shouldn't be seeing 2 year old comments which are probably now invalid anyway.

As for creating new comments, they could automatically be delegated to the last commit.

👍

@garfieldnate
Copy link

I want this. 👍
I think the mechanism for keeping comments across commits could roughly be the way a good implementation of patch works: using fuzzy matching. There are several implementations for it in Google diff/match/patch here.

  1. User comments line. Line text and location is saved as match for comment.
  2. User changes line during commit.
  3. Find the new location of the comment using match_main(new_commit_text, old_line_text, old_line_location). If the line is not found, then the comment is "dangling" and either delete it or mark it for user action, depending on user preferences.

It would also be nice to keep a complete comment history for the file, and to move comments between files when their commented lines are moved (does git provide this info?).

Documentation for match_main is here. The research question here is determining acceptable values of diff_match_patch.Match_Threshold, which determines the fuzziness of the text match, and diff_match_patch.Match_Distance, which determines the fuzziness of the location match.

@marnen
Copy link

marnen commented Jul 3, 2015

Another +1. I'm a Google Summer of Code mentor, and I'd like to do a general code review on my student's repo. This is the exact feature I need to do that.

@omidkrad
Copy link

omidkrad commented Jun 8, 2016

Wow, this was requested 2 years ago and there is exactly no improvement on this yet!

This is the workflow I'm looking for:

  • Find and view a file under a repository
  • Press 'y' to jump to the blob
  • Click on a line number and add a comment

Also it would be nice to make the jump to blob (shortcut key 'y') feature more obvious, maybe by adding a button for it in the UI.

@yihui
Copy link

yihui commented Nov 5, 2016

Another 👍. This will be very helpful for reviewing papers and books.

@nsheff

This comment has been minimized.

@chchrist

This comment has been minimized.

8 similar comments
@freewind
Copy link

freewind commented Feb 8, 2017

👍

@joshilewis
Copy link

+1

@xiaohanyu
Copy link

+1

@jleben
Copy link

jleben commented Mar 10, 2017

+1

@chookie
Copy link

chookie commented May 23, 2017

👍

@JohnRoesler
Copy link

👍

@boooklet
Copy link

+1

@stenioaraujo
Copy link

👍

@jmehnle
Copy link

jmehnle commented Jul 10, 2017

Here's a workaround that involves creating an "empty" branch and then creating a pull request to merge the desired branch into the empty branch: http://astrofrog.github.io/blog/2013/04/10/how-to-conduct-a-full-code-review-on-github/

@rumaniel

This comment has been minimized.

@alecbz
Copy link

alecbz commented Oct 24, 2017

Please don't reply to an issue just to say '+1': it sends a notification to everyone subscribed to the issue. Instead, just react to the issue's initial comment with the 👍 emoji.

plus_1_github

@cmagnus

This comment has been minimized.

@TPS TPS added the code review label Mar 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests