[Kde-pim] Review Request 118699: Support for referenced collections.

Christian Mollekopf chrigi_1 at fastmail.fm
Wed Jun 18 08:45:23 BST 2014



> On June 16, 2014, 9:46 a.m., Dan Vrátil wrote:
> > server/src/collectionreferencemanager.cpp, line 63
> > <https://git.reviewboard.kde.org/r/118699/diff/2/?file=280996#file280996line63>
> >
> >     Should this be if (isReferenced(col))?

No, this check exists to ensure we only reset the flag once no collection any longer references the collection.


> On June 16, 2014, 9:46 a.m., Dan Vrátil wrote:
> > server/src/collectionreferencemanager.cpp, line 38
> > <https://git.reviewboard.kde.org/r/118699/diff/2/?file=280996#file280996line38>
> >
> >     This must be protected by a mutex, or you must call this method from the main thread before Akonadi starts listening for connections.
> >     
> >     Alternatively, the Manager could be owned by AkonadiServer, like CacheCleaner() to avoid having singleton at all.

If the instance is afterwards accessed over the singleton it's effectively still a singleton, so IMO not an improvement. I therefore opted to protect the instance() call with a mutex.


- Christian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118699/#review60174
-----------------------------------------------------------


On June 13, 2014, 5:52 p.m., Christian Mollekopf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118699/
> -----------------------------------------------------------
> 
> (Updated June 13, 2014, 5:52 p.m.)
> 
> 
> Review request for Akonadi.
> 
> 
> Repository: akonadi
> 
> 
> Description
> -------
> 
> This patch add support for referencing disabled collections so they show up in a single session.
> I think it's complete and functional, but I didn't get around using some singletons and perhaps introduced a couple unwanted dependencies. Feel free to point out how to do it better =)
> 
> ModifyHandlerTest: Removed duplicate test.
> 
> 
> Fixed listhandlertest.
> 
> Or is the order not guaranteed?
> 
> Support for temporary referenced collections.
> 
> 
> Option to avoid db population in fakedatastore.
> 
> This allows tests to avoid the population by the xml data so they can
> populate the store themselves.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 136d47d53e55a4b2116e2a8509001f3121a58341 
>   libs/protocol_p.h 7bb1fccf11da7971cf4f7c8e304c323e84399704 
>   server/CMakeLists.txt b67d467d853a8cd69502aa161cf4bb31474b40af 
>   server/src/akonadi.cpp cdd412f0aba73070e42d554f26edf09aa8af1793 
>   server/src/collectionreferencemanager.h PRE-CREATION 
>   server/src/collectionreferencemanager.cpp PRE-CREATION 
>   server/src/connection.h 951f4167f8dc199245c2f975ef3a8db94fbd88ac 
>   server/src/connection.cpp aab375f1e8decd32c3d351b854630c494d83fbc6 
>   server/src/handler/list.cpp c6a3b6f19e3f3e46034f495b018ee0bc0f6f400b 
>   server/src/handler/modify.h 0e8a0e9a40f14cfa4c6b381d120d51c760bfcddf 
>   server/src/handler/modify.cpp 3ce094bdd7ad3fe7f5f688503cfafdce195a814c 
>   server/src/handlerhelper.h dae6f2c149c2343ce08e21e791f3b9f4968c59a7 
>   server/src/handlerhelper.cpp d3ce33fb2fea8787b7df133296e9d2545b385425 
>   server/src/notificationmanager.h 30fabebe0712e605397a53452777e14c60dd3dfd 
>   server/src/notificationmanager.cpp 00eb2ae031c2755098546eadd0a7e349e426475a 
>   server/src/notificationsource.h 9eec806a73307de524be0fe145f0c16111ba604b 
>   server/src/notificationsource.cpp 152706416e10ec9dcce632baa75a02c803abd3bd 
>   server/src/preprocessormanager.cpp 517a55ae68881dc9e84bc425af39e3068cedca89 
>   server/src/storage/akonadidb.xml 70bde765b8f161f8c473c3323994942937baadec 
>   server/tests/unittest/CMakeLists.txt f2f0aaabbc69244013771457c5bc0ab2482241e2 
>   server/tests/unittest/collectionreferencetest.cpp PRE-CREATION 
>   server/tests/unittest/fakeakonadiserver.h fefbb8a30ee18aa72ae0cf41f39aa07ce9c89635 
>   server/tests/unittest/fakeakonadiserver.cpp b1bb9aee741435ec11f11232c74fa4dd72c7e054 
>   server/tests/unittest/fakedatastore.h 9add4e425ff309414b814e098514318c3e0d975e 
>   server/tests/unittest/fakedatastore.cpp 4af3375a3b0d7cde2f8c0cbd784942f32c9cbbb6 
>   server/tests/unittest/listhandlertest.cpp 35bf0da2ee18f0fd98d7ba4e738227ba93df4141 
>   server/tests/unittest/modifyhandlertest.cpp a4b00414e591f9107a2454a8e5bf1a5bc194496c 
> 
> Diff: https://git.reviewboard.kde.org/r/118699/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Christian Mollekopf
> 
>

_______________________________________________
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/


More information about the kde-pim mailing list