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
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
