CODETOOLS-7900327: Change webrev to use 'trees' instead of 'forest'
Iris Clark
iris.clark at oracle.com
Tue Mar 18 16:19:32 UTC 2014
Hi, Daniel.
Please send me an example of the new functionality you'd like to highlight. I'll include it on this page:
http://openjdk.java.net/guide/webrevHelp.html
Thanks,
iris
-----Original Message-----
From: Jonathan Gibbons
Sent: Tuesday, March 18, 2014 8:21 AM
To: Daniel Fuchs; webrev-dev at openjdk.java.net
Subject: Re: CODETOOLS-7900327: Change webrev to use 'trees' instead of 'forest'
On 03/18/2014 06:57 AM, Daniel Fuchs wrote:
> Hi Jon,
>
> On 3/18/14 2:37 AM, Jonathan Gibbons wrote:
>> Daniel,
>>
>> There are still references to the forest extension and to fstatus in
>> the comments that should probably be removed or updated.
>>
>> For example,
>>
>> 1365 #
>> 1366 # forest extension is still being changed. For instance the
>> output
>> 1367 # of fstatus used to no prepend the tree path to filenames, but
>> 1368 # this has changed recently. AWK code below does try to handle
>> both
>> 1369 # cases
>> 1370 #
>>
>>
>> 1379 #
>> 1380 # There is a bug in the output of fstatus -aC on recent
>> versions: it
>> 1381 # inserts a space between the name of the tree and the filename
>> of the
>> 1382 # old file. e.g.:
>> 1383 #
>>
>> Also, it looks like 1396-1411 are working around the bug described in
>> that comment.
>
> Well - I'm not feeling bold enough to try and change that code.
> I have tested the new webrev with renamed files, both in the same dir
> and in different dirs and in subrepos, and it seems to still do the
> right thing.
>
> Therefore - I have simply updated the comments so that someone who
> would want to attempt and cleanup this code would understand that part
> of it is historical technological debt dating from the 'forest'
> time:
>
> http://cr.openjdk.java.net/~dfuchs/webrev_CODETOOLS-7900327/webrev.02/
>
> I hope that can be enough for now; if it ain't broke... ;-)
>
> best regards,
>
> -- daniel
>
>>
>> -- Jon
>>
>>
>> On 03/17/2014 06:26 PM, Jonathan Gibbons wrote:
>>> Daniel,
>>>
>>> I can commit the change for you, but I'm not familiar with the
>>> webrev code so I'd like to see some general approval from folk that
>>> are.
>>>
>>> -- Jon
>>>
>>> On 03/17/2014 02:38 AM, Daniel Fuchs wrote:
>>>> Hi,
>>>>
>>>> I would like to propose a patch for fixing
>>>>
>>>> CODETOOLS-7900327: Change webrev to use 'trees' instead of 'forest'
>>>> https://bugs.openjdk.java.net/browse/CODETOOLS-7900327
>>>>
>>>> webrev:
>>>> http://cr.openjdk.java.net/~dfuchs/webrev_CODETOOLS-7900327/webrev.
>>>> 00/
>>>>
>>>> (I will need a sponsor)
>>>>
>>>> This patch changes the behavior of the '-f' option, so that it
>>>> relies on 'trees' [1] instead of 'forest'.
>>>> For this to work your forest needs to have been configured for trees.
>>>> This happens automatically if you used 'hg tclone' to clone it, but
>>>> otherwise you can call 'hg tconfig --set --walk --depth' to
>>>> configure it (you can use 'hg tlist' to see the list of trees that
>>>> have been configured).
>>>>
>>>> The main changes in the patch (trickiest) comes from the fact that
>>>> whereas hg f<cmd> used to print '[<relative-path>]'
>>>> hg t<cmd> now prints '[<full-path>]:'
>>>>
>>>>
>>>> I tested the patch with the following options:
>>>>
>>>> [all files edited, no commit]:
>>>>
>>>> webrev -fN
>>>>
>>>> [all files committed, no edits]:
>>>>
>>>> webrev -f
>>>>
>>>> best regards,
>>>>
>>>> -- daniel
>>>> [1] http://openjdk.java.net/projects/code-tools/trees/
>>>>
>>>>> Hi Daniel;
>>>>>
>>>>> Since it looks like several people have tried your patch and seem
>>>>> to find it works for them then perhaps you should just create a
>>>>> CODETOOLS issue and propose it to this list (webrev-dev) for
>>>>> sponsorship.
>>>>>
>>>>> I haven't had a chance to try it yet but likely wouldn't have
>>>>> anything to add by my review (and I am not a committer/reviewer on
>>>>> CODETOOLS) so I would encourage you to go ahead with proposing it
>>>>> yourself.
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Mike
>>>>
>>>>
>>>>
>>>
>>
>
Daniel,
I'll take a look at your updated webrev, and file an RFE for the potential cleanup.
-- Jon
More information about the webrev-dev
mailing list