[Kde-pim] Re: Review Request: Emit changed signals for subentities of an explicitly monitored collection

[Christian Mollekopf]

On Tue, 03 May 2011 11:57:25 +0200, Stephen Kelly <steveire at gmail.com>  
wrote:

> Stephen Kelly wrote:
>
>> Christian Mollekopf wrote:
>>
>>> On Monday, April 25, 2011 10:59:57 AM Stephen Kelly wrote:
>>>> Stephen Kelly wrote:
>>>> > It will need unit tests though, and I won't get a chance to write  
>>>> them
>>>> > before Monday at the earliest.
>>>> >
>>>> > - Stephen
>>>>
>>>> Please test the attached patch which includes the unit tests. I'll
>>>> commit it after you test & review.
>>>
>>> I don't know why, but with your patch my akonadi goes berserk.
>>> I get lots of "RID mismatch" messages and my imap account started to
>>> generate invalid email items in an endless loop.
>>>
>>> I attached my console output. Tell me if you need something else for
>>> debugging.
>>
>> I might have reproduced this. I got a RID mismatch message in the  
>> maildir
>> resource, which is code I don't know unfortunately.
>>
>> I'll look into it a bit though.
>>
>
> New patch attached. I think the ChangeRecorder stuff might still be  
> broken.
> Will have to review that with Volker later.
>

The only thing I noticed is that I get "Invalid Collection in prefetch"
messages from isCollectionMonitored() when I delete a collection with  
items in it (i.e. a subcollection of the monitored collection).
I don't know which exact call it is, but I think it is to be expected as  
the parentCollection is already invalidated (according to one of your  
comments).

 From a behaviour POV it works perfectly together with the ETM patches.

Regarding the TODO: q->setCollectionMonitored( Collection( msg.uid() ),  
false );
I think you can just silently remove it from collections, as the  
collectionMonitored( collection, monitored ); signal is not needed.
It is also not useful to keep it in collections.
Maybe add a call to cleanOldNotifications to be sure that there are not  
more waiting (not sure if this is possible).

I had a look at the monitor part of the patch, and it looks all ok IMO.

So from my side you can commit, the minor issues can be checked later.


-- 
Using Opera's revolutionary email client: http://www.opera.com/mail/
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/