Mike, Looks good for me
(not a reviewer) -Dmitry On 2013-10-17 02:31, 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 """ inside the "Bug id" header entry. >> >> I have fixed this. It was being double escaped. ie. &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 >>>>>> >>>> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
