Looks fine.

Thanks
Max

On 10/17/13 6:31 AM, Mike Duigou wrote:
Reminder:

http://cr.openjdk.java.net/~mduigou/JDK-8026062/2

needs final review.

Mike

On Oct 14 2013, at 11:56 , Mike Duigou <mike.dui...@oracle.com> wrote:


On Oct 11 2013, at 18:03 , Weijun Wang wrote:

The webrev shows "&quot;" inside the "Bug id" header entry.

I have fixed this. It was being double escaped. ie. &amp;quot;

Updated webrev:

http://cr.openjdk.java.net/~mduigou/JDK-8026062/2



Also, the following headers look a little suspicious:

  Compare against: ssh://hg.openjdk.java.net/jdk8/tl-gate
  Compare against version: -2

I usually compare with -2 when -1 is an mq patch and I also have "-N". But the 
"compare against" line seems to show you are comparing to -2 of the remote repo. That 
would include the last changeset.

This is because I have

[mq]secret = True

in my .hgrc file. (secret prevents me from accidentally committing mq patches 
to a non-jcheck repo). It may look a little unfamiliar but it is not a 
consequence of my webrev changes.


It's so cool reviewing webrev itself without looking at the actual code 
changes. :)

Thank you for the review!

Mike


Thanks
Max

On 10/12/13 8:22 AM, Mike Duigou wrote:

On Oct 11 2013, at 14:56 , Bradford Wetmore wrote:

It never worked like it did in Teamware.

Thanks Brad, that's what I thought reading the source. It appeared that it had 
never been implemented for mercurial. I have removed support for -l

Here's an updated webrev (generated with itself!):

http://cr.openjdk.java.net/~mduigou/JDK-8026062/1

In addition to removing support for -l it also cleans up options handling, fixes 
incorrect escaping of &# numeric character entities (pointed out by John Rose), 
and adds some documentation to the -? help.

Mike



Brad

On 10/11/2013 11:33 AM, Mike Duigou wrote:
I should also ask if anyone is using the -l option. I would like to delete it 
as well as it offers no particular value for mercurial (that I can tell).

Mike

On Oct 10 2013, at 17:20 , Weijun Wang wrote:



On 10/10/13 1:13 PM, Mike Duigou wrote:

On Oct 9 2013, at 22:11 , Weijun Wang wrote:

Some of us still use wxfile to generate a single webrev for code changes in 
multiple repos. Is there another way to do it?

Yes, you can pipe a simple file list to webrev to have it use that list of 
files. I removed wxfile support in part because it's not documented anywhere 
and there are alternatives. If you can confirm that the alternatives are 
adequate I would very much appreciate being able to go ahead with removing 
wxfile support.

I tried to create a file list and pipe it to "webrev.ksh -N -r -1" but seems 
not working. Am I using the wrong options?

Thanks
Max




Reply via email to