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