Review Request: This patch adds the Storage class for Plasma::DataEngine caching. This current implementation caches to disk. Soon, it will be akonadi.

[Marco Martin]

> On 2010-07-10 08:35:01, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/datacontainer.cpp, line 153
> > <http://reviewboard.kde.org/r/4548/diff/1/?file=30537#file30537line153>
> >
> >     needsCacheing()?

sounds pretty awful as well.
what about replacing all references to "cache" in the api to storage?
so this would become needsStorage or needsToBeStored


> On 2010-07-10 08:35:01, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/datacontainer.h, line 127
> > <http://reviewboard.kde.org/r/4548/diff/1/?file=30536#file30536line127>
> >
> >     apidox are required. method should be const.

oh, yeah, how i could have not seen this :)


> On 2010-07-10 08:35:01, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/dataengine.h, line 348
> > <http://reviewboard.kde.org/r/4548/diff/1/?file=30538#file30538line348>
> >
> >     in the wrong place in the header: it has separated removeAllData from its apidox. setEnableCache also requires apidox.
> >     
> >     i wonder, also, if we'll end up with use cases for a "global" setEnableCache which will dis- or en-able caching for all sources.

what about regex matching for the sources? maybe overkill...


> On 2010-07-10 08:35:01, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/datacontainer.h, lines 225-226
> > <http://reviewboard.kde.org/r/4548/diff/1/?file=30536#file30536line225>
> >
> >     both of these members must be in the private member. as it is, this breaks binary compatibility!

To avoid those things in the future, give a read to
http://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B
is a good list of do and don't


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4548/#review6459
-----------------------------------------------------------


On 2010-07-08 16:47:24, Brian Pritchett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4548/
> -----------------------------------------------------------
> 
> (Updated 2010-07-08 16:47:24)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> DataEngines can mark or unmark their sources to be cached with void DataEngine::setEnableCache(const QString &source, bool cache). If the DataEngine has implemented their own source by inheriting DataContainer, then DataContainer::setEnableCache(bool cache) will work.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/CMakeLists.txt 1147556 
>   /trunk/KDE/kdelibs/plasma/data/operations/storage.operations PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/datacontainer.h 1147556 
>   /trunk/KDE/kdelibs/plasma/datacontainer.cpp 1147556 
>   /trunk/KDE/kdelibs/plasma/dataengine.h 1147556 
>   /trunk/KDE/kdelibs/plasma/dataengine.cpp 1147556 
>   /trunk/KDE/kdelibs/plasma/private/dataengine_p.h 1147556 
>   /trunk/KDE/kdelibs/plasma/private/storage.h PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/private/storage.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/4548/diff
> 
> 
> Testing
> -------
> 
> I have tested it with the microblogging dataengine/plasmoid.
> 
> 
> Thanks,
> 
> Brian
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20100710/716b1e1c/attachment.htm