Last Comment Bug 782717 - Link from reviews back to the patch
: Link from reviews back to the patch
Status: NEW
Classification: Developer Infrastructure
Component: Extensions: Splinter (show other bugs)
: Production
: All All
P5 enhancement (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
Depends on:
  Show dependency treegraph
Reported: 2012-08-14 11:15 PDT by David Teller [:Yoric] (please use "needinfo")
Modified: 2016-06-16 15:04 PDT (History)
3 users (show)
See Also:
Due Date:
QA Whiteboard:
Iteration: ---
Points: ---


Description User image David Teller [:Yoric] (please use "needinfo") 2012-08-14 11:15:34 PDT
Let's consider the following patch:

if (foo) {
 // ... do something
// ...
if (foo) {
 // ... do something else

When a reviewer adds a comment pertaining to line |if(foo) {|, the context is generally lost. What the submitter will see is something along the lines of

+ if(foo) {

This is not very informative (especially when the reviewer is monosyllabic, and we have quite a few of these) and the submitter has to guess whether the comment was related to the first occurrence of |if(foo) {| or the second. This can get even worse when the only context displayed after the review is some empty line.

We could make this much nicer on the users by making the following change:
- in the message, each line starts with a line number;
- each line number links back to the corresponding line in the Splinter Review vies of the patch.

As follows

l. 23 +   if(foo) {

Note You need to log in before you can comment on or make changes to this bug.