[Python-Dev] View tracker patches with ViewVC?

[Terry Reedy]

On 7/27/2010 11:52 AM, Reid Kleckner wrote:

>> Let me repeat me original question: Would it be feasible to add a [view]
>> button that I could click to get a nice view of a patch, such as provided by
>> ViewVC?
>
> How are you proposing to use ViewVC to view the patch?  I'd think that
> you'd have to commit it first, unless it has some functionality that
> I'm unaware of.

The point of the tracker is to review patches *before* they are 
committed. The information is the same before and after it is committed. 
The question of 'feasible' asks whether ViewVC is inextricably linked to 
an svn repository. If it is, either technically or legally, then it is.

> Anyway, one uses Rietveld mostly via upload.py, not the form above.

I had never heard of upload.py. Like Rietveld, it is not mentioned in 
the tracker docs. I see it mentioned on he rietveld page linked below. 
The page says it requires 2.5 (which newcomers will not have laying 
aroung)and only says how to run it on *nix, so I would not be surprised 
if it were to not run on Windows due to using *nix only functions.

> Would the ViewVC functionality you are proposing look like this?
> http://svn.python.org/view/python/branches/release27-maint/Demo/classes/Vec.py?r1=82503&r2=83175&pathrev=83175

Yes, I presume that is what some people use to do post-commit reviews.

> Rietveld's differ is smarter (it does intra-line diffs)

Ok, that would be even better.

> and the inline comments there are a lot better

than out of context comments in a tracker message.
>
> It's true that the workflow isn't really described anywhere,

So don't expect ignorant folks like me to follow it ;-).

> so I'll try to outline it in detail here.

Like code patches, doc patches in here-today, gone-tomorrow email is not 
too useful. Better to put it the doc or in a tracker issue for further 
revision.

> Author's steps to upload a patch and create an issue:
> - Discuss issue in the tracker
> - Hack away at solution in svn checkout
> - When done, run `upload.py` (no args creates a new issue and prints URL)
> - When prompted, enter Google account credentials
> - When prompted, enter the issue title you want to give it, probably
> by pasting in the tracker title plus IssueXXX
> - I always check the diff on Rietveld to make sure it looks good to me
> before sending
> - Go to the URL printed and click 'Start Review' to send mail
>
> Reviewer's steps to add review comments:
> - Receive mail, click URL to open issue
> - Click the link to the first file, and read through the colored diff,
> using 'n' to scroll down and 'j' to go to the next file.
> - To make a comment, double click the line you want to comment on.
> This is the most unintuitive part to beginners.
>    - Enter the comment in the textbox that appears.
> - Repeat until done reading the diff, then go back to the issue page
> and click 'Publish+Mail Comments'
>
> Author's steps to respond to comments:
> - Open the files in the issue
> - Read through the comments ('N' skips from comment to comment)
> - Apply fixes, reply to each comment
> - Run `upload.py -i<issue#>` to add a new patch with your fixes.
> - Reply by clicking 'Publish+Mail Comments' to let your reviewer know
> that you've addressed the comments
>
> Repeat ad nauseum until reviewer is happy, then commit.
>
> ===
>
> Not sure why I spelled that all out when these docs exist:
> http://code.google.com/p/rietveld/wiki/CodeReviewHelp

Because
a) someone reading http://wiki.python.org/moin/TrackerDocs/
would not know it exists and
b) because the above is *much* cleared than that page.

Let me try to explain better what my original post was about.

There have been many posts on pydev about the need for more patch 
reviews. A couple of people have even made 5 for 1 offers to encourage 
more reviews and have emphasized that even partial reviews are more 
helpful than none. In response to these appeals, including the last 
point, I recently starting doing more patch reviews than in the past. 
Simple diffs that replace block A with block B I can read fine. 
Sufficiently complex diffs, say with 10 interlaced changed, I cannot.

A couple of days ago, I got an email that a doc issue I opened was now 
closed with revxxxxx, patch never posted to the tracker. I followed the 
link, saw the [text] button, and got the page with the colored, 
side-by-side display. I thought, "Wow, I wish I could see patches like 
this while reviewing, before they are committed!". And to do so just as 
easily, with one click. (As it turns out, the patch needed review 
because it had a minor error, but that is another issue.)

Rietveld may be a bit better, but it potentially is a lot of work to get 
that bit better, especially the firs time. And it may be overkill for 
small to medium patches.

So what I am suggesting it this: to get more and better patch reviews, 
especially from new reviewers, make it as easy to get an improved view 
of diffs as it is now to view the raw file.

-- 
Terry Jan Reedy