[Kde-pim] Review Request: Store payload data in files

[Andras Mantia]

> On 2009-02-04 05:28:28, Volker Krause wrote:
> > trunk/kdesupport/akonadi/server/src/storage/parthelper.h, line 58
> > <http://reviewboard.kde.org/r/13/diff/4/?file=151#file151line58>
> >
> >     is that actually used anywhere?

No, I added there because it was in the public API for the Part class.


> On 2009-02-04 05:28:28, Volker Krause wrote:
> > trunk/kdesupport/akonadi/server/src/storage/parthelper.h, line 60
> > <http://reviewboard.kde.org/r/13/diff/4/?file=151#file151line60>
> >
> >     seems unused as well

Same reason, part of the API.


> On 2009-02-04 05:28:28, Volker Krause wrote:
> > trunk/kdesupport/akonadi/server/src/storage/parthelper.cpp, line 29
> > <http://reviewboard.kde.org/r/13/diff/4/?file=152#file152line29>
> >
> >     Good enough for now, but I think we want something more elaborate here in the long run (config settings, auto-selection on part size, etc.).

Sure, this is more about testing and for us developer to actually have something to start with.


> On 2009-02-04 05:28:28, Volker Krause wrote:
> > trunk/kdesupport/akonadi/server/src/storage/parthelper.cpp, line 52
> > <http://reviewboard.kde.org/r/13/diff/4/?file=152#file152line52>
> >
> >     Do we need the "_data" suffix at all?

Technically no, the id is just fine. I just found it strange to have file names consisting only of numbers, but as this is not for the users, we can just go on with the id's only.


> On 2009-02-04 05:28:28, Volker Krause wrote:
> > trunk/kdesupport/akonadi/server/src/storage/parthelper.cpp, line 98
> > <http://reviewboard.kde.org/r/13/diff/4/?file=152#file152line98>
> >
> >     Seeing this code repeated a few times, we probably want a helper method fileNameForPart() or something like that?

Agreed.


> On 2009-02-04 05:28:28, Volker Krause wrote:
> > trunk/kdesupport/akonadi/server/src/storage/parthelper.cpp, line 155
> > <http://reviewboard.kde.org/r/13/diff/4/?file=152#file152line155>
> >
> >     This assumes that value is the id, which is not always true. One place where this method might be used is removing an item and all its parts. In this case column would be "pimItemId" and value the item id.

Thanks, I misunderstood the usage of this code.


> On 2009-02-04 05:28:28, Volker Krause wrote:
> > trunk/kdesupport/akonadi/server/src/storage/parthelper.cpp, line 171
> > <http://reviewboard.kde.org/r/13/diff/4/?file=152#file152line171>
> >
> >     Maybe call loadData(Part&) here instead?

Sure, "historical" reasons, I wrote this method first. :)


- Andras


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


On 2009-02-04 04:27:40, Andras Mantia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/13/
> -----------------------------------------------------------
> 
> (Updated 2009-02-04 04:27:40)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Preliminary implementation of storing the payload data in files instead of the database. It work for me, but it is not well tested. I'd like to get a general feedback about the idea how it is implemented (or anything else, if you have a comment). The basic idea is to use a helper class when dealing with Part objects, and the helper class decides where to store/load the data.
> 
> 
> Diffs
> -----
> 
>   trunk/kdesupport/akonadi/server/CMakeLists.txt 921033 
>   trunk/kdesupport/akonadi/server/src/akonadi.cpp 921033 
>   trunk/kdesupport/akonadi/server/src/akonadiconnection.h 921033 
>   trunk/kdesupport/akonadi/server/src/cachecleaner.cpp 921033 
>   trunk/kdesupport/akonadi/server/src/global.h 921033 
>   trunk/kdesupport/akonadi/server/src/handler/append.cpp 921033 
>   trunk/kdesupport/akonadi/server/src/handler/copy.cpp 921033 
>   trunk/kdesupport/akonadi/server/src/handler/fetch.cpp 921033 
>   trunk/kdesupport/akonadi/server/src/handler/store.cpp 921033 
>   trunk/kdesupport/akonadi/server/src/storage/akonadidb.xml 921033 
>   trunk/kdesupport/akonadi/server/src/storage/datastore.cpp 921033 
>   trunk/kdesupport/akonadi/server/src/storage/entities-header.xsl 921033 
>   trunk/kdesupport/akonadi/server/src/storage/parthelper.h PRE-CREATION 
>   trunk/kdesupport/akonadi/server/src/storage/parthelper.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/13/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andras
> 
>

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