My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 93: Display inline file comments below change message in ChangeScreen
42 people starred this issue and may be notified of changes. Back to list
Status:  Released
Owner:  ----
Closed:  Nov 2013
Cc:  j...@android.com, zachw...@gmail.com

Blocked on:
issue 348


Sign in to add a comment
 
Reported by code-rev...@gtempaccount.com, Sep 24, 2009
Reported by Jeff Hamilton <jham@android.com> on Tue Mar 03 12:12:05 PST 2009
Source: JIRA GERRIT-93
Affected Version: 2.0.6

It would be really nice if the change description page for a change included
the inline comments in the Messages section, like it does for the email. Just
looking at a single message doesn't convey what the reviewing was trying to
communicate if there are inline comments.
Sep 24, 2009
#1 code-rev...@gtempaccount.com
Comment by Zach Wily <zach@zwily.com> on Sun Mar 29 14:52:07 PDT 2009

If we do this, it would also be nice to be able to select a block of lines to
comment on, so when the code comment is shown (both in the UI and in email) it
can show more than 1 line of code.
Sep 24, 2009
#2 code-rev...@gtempaccount.com
Comment by Shawn Pearce <sop@google.com> on Sun Mar 29 20:18:41 PDT 2009

I think what Zach is asking for is interesting, but is unrelated to this issue
request.  Though if we do what Zach is talking about, it does complicate the
display a little as we would want to quote a block of lines in the Messages
section, not just one line.
Sep 24, 2009
#3 code-rev...@gtempaccount.com
Comment by Zach Wily <zach@zwily.com> on Mon Apr 20 20:51:47 PDT 2009

Well, I think it's tangentially related for that reason. If comments only
apply to a single line of code (like they do now, essentially), then the
comment displayed inline either only includes the single line of code or some
hardcoded number of lines of context. Allowing comments to target multiple
lines would make showing the comments inline in the Messages section more
useful.

I believe that Reviewboard does it this way. You can also reply to inline
comments from within the Messages section, but now I'm getting really off-
topic...

I filed GERRIT-141 to track the multiple-lines feature.
Sep 24, 2009
#4 sop+code@google.com
(No comment was entered for this change.)
Owner: ---
Sep 24, 2009
#5 sop+code@google.com
(No comment was entered for this change.)
Cc: j...@android.com zachwily
Apr 24, 2010
#6 sop@google.com
(No comment was entered for this change.)
Status: Accepted
Aug 13, 2010
Project Member #7 mf...@codeaurora.org
How does this relate to GERRIT-396.  It seems like having just the comment count was considered more appropriate?
Sep 7, 2010
#8 sop@google.com
 Issue 717  has been merged into this issue.
Sep 7, 2010
#9 e...@google.com
this would be a real help when doing archeology, trying to find out why a change ended up the way it did.
Sep 29, 2010
#10 sir...@gmail.com
The lack of inline comments is the one feature that has unfortunately held back my company from switching over fully from reviewboard to gerrit. It'd be great if someone on the Gerrit team could provide an update about whether this is on the roadmap, and, if not, how a third party like us could get started adding this feature.

Oct 4, 2010
Project Member #11 mf...@codeaurora.org
If you want to get started on this, here is the patch to add the minimal summary from  Issue 396 .  It would likely be trivial to add your own summary format here in this same function.  In fact, you might simply copy or refactor the code from the email sending class into a utility function and call it from here.

https://review.source.android.com/#patch,sidebyside,15275,1,gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java

May 19, 2011
Project Member #12 nas...@grainawi.org
Current thoughts on how to do this are to add a string with the file path and then "Line ##: This is the first 60 chars of the inline comment". We would make those linkable into the diff (as  Issue 348  suggests).

We need to fix old change comments to support this though. On database upgrade we should query the comments from the db and do a userid and timestamp correlation.
Blockedon: 348
Dec 7, 2011
Project Member #13 bklarson@gmail.com
 Issue 1117  has been merged into this issue.
Nov 20, 2013
#14 sop@google.com
https://gerrit-review.googlesource.com/51826
Status: ChangeUnderReview
Cc: -jham%android.com@gtempaccount.com j...@android.com
Blockedon: -gerrit:348 gerrit:348
Nov 21, 2013
#15 sop@google.com
(No comment was entered for this change.)
Status: Submitted
Labels: FixedIn-2.9
Jul 18, 2014
Project Member #16 edwin.ke...@gmail.com
(No comment was entered for this change.)
Status: Released
Today (3 hours ago)
#19 Renijuli...@gmail.com
It'd be great if someone on the Gerrit team could provide an update about whether this is on the roadmap, and, if not, how a third party like us could get started adding this feature. [http://www.technologic.xyz/]
[http://mela.indonesiaz.com/]
[http://hargahape.blogdetik.com/]
[http://hargahape.blog.com/]
Sign in to add a comment

Powered by Google Project Hosting