[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix issue #2829

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: 2007-10-25 15:13:50 CEST

Hi Senthil,

Thanks for the revised patch. I have couple of concerns right now.

In 'determine_merges_performed', we record inherited mergeinfo for
siblings of skipped path, which is technically correct but confuses the
meaning of 'determine_merges_performed' we may have to do it via small
utility function of some sort.

I have to see why r26803 has called update_wc_mergeinfo twice for
'target' that causes the 'target' to get 'requested mergeinfo' always.

With regards
Kamesh Jayachandran
Senthil Kumaran S wrote:
> Hi Kamesh,
>
> Kamesh Jayachandran wrote:
>> My review comments are as follows,
>
> Thank you for your comments. I am attaching an improved patch along
> with this mail. Please review this patch and let me know your
> comments. Some of the things which you have mentioned in your previous
> comments are not corrected in this patch which are as follows:
>
>>> @@ -1005,13 +1005,14 @@
>>
>>> })
>>
>>> expected_disk = wc.State('', {
>>
>>> '' : Item(props={SVN_PROP_MERGE_INFO : '/A/B:1,3'}),
>>
>>> - 'F' : Item(),
>>
>>> - 'lambda' : Item("This is the file 'lambda'.\nchange lambda.\n"),
>>
>>> + 'F' : Item(props={SVN_PROP_MERGE_INFO : '/A/B/F:3'}),
>>
>>> + 'lambda' : Item(contents="This is the file 'lambda'.\nchange
>>> lambda.\n",
>>
>>> + props={SVN_PROP_MERGE_INFO : '/A/B/lambda:3'}),
>>
>>> })
>>
>>
>> Here '' should have a mergeinfo of '/A/B:1'
>> 'F' and 'lambda' should have a mergeinfo of '/A/B:1,3'
>
> Here '' still has mergeinfo '/A/B:1,3'
>
>>> expected_status = wc.State(I_path, {
>>
>>> '' : Item(status=' M'),
>>
>>
>> As per above point here '' should not have prop modified.
>
> Not corrected yet.
>
>>> @@ -1834,7 +1835,7 @@
>>
>>> })
>>
>>> expected_disk = wc.State('', {
>>
>>> '' : Item(props={SVN_PROP_MERGE_INFO : '/A/B/F:2'}),
>>
>>
>> Here '' should have no merge info as some of its child got skipped of
>> merge.
>>
>>> @@ -1881,7 +1882,8 @@
>>
>>> expected_disk = wc.State('', {
>>
>>> '' : Item(props={SVN_PROP_MERGE_INFO : '/A/B/F:2'}),
>>
>>
>> Here '' should have no merge info as some of its child got skipped of
>> merge.
>
> Still this exists.
>
> But with this patch all the test cases pass in all the 3 layers.
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Oct 25 15:14:36 2007

This is an archived mail posted to the Subversion Dev mailing list.