[Kde-pim] Review Request: Refactor Akonadi ical resource classes to allow inheritance by KAlarm resource

[Kevin Krammer]

> On 2009-07-19 08:43:00, Kevin Krammer wrote:
> > /trunk/KDE/kdepim/akonadi/resources/ical/icalresourcebase.h, line 73
> > <http://reviewboard.kde.org/r/1057/diff/1/?file=8638#file8638line73>
> >
> >     if changed == true it means added?
> 
>  wrote:
>     Ah! Well spotted - I'll change that (or do I mean add that? ;-) )

Maybe use an enum instead, e.g. CheckForAdded, CheckForChanged


> On 2009-07-19 08:43:00, Kevin Krammer wrote:
> > /trunk/KDE/kdepim/akonadi/resources/ical/notes/notesresource.cpp, line 44
> > <http://reviewboard.kde.org/r/1057/diff/1/?file=8645#file8645line44>
> >
> >     Doesn't that overwrite any value previously configured by the user?
> 
>  wrote:
>     This just implements what the Notes option does in the current code. Doing this differently would change the way the existing Notes resource works.

Then we should fix it there as well. The notes resources has not been shipped yet, so we don't have too keep any compatibility.
I think what was intended by that code is to change the default, not the actual value.
A combination of KCoreConfigSkeleton::findItem() and KConfigSkeletonItem::PathItem::setDefaultValue() should work


- Kevin


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


On 2009-07-18 23:27:20, David Jarvie wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1057/
> -----------------------------------------------------------
> 
> (Updated 2009-07-18 23:27:20)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> This is a refactoring of ICalResource discussed with Volker and Kevin at Akademy. It provides an ICalResourceBase class which is suitable for inheritance by KAlarm for its Akonadi resource. It also separates out the specialisations required by KNotes into another class inherited from ICalResource, rather than having KNotes code interspersed with the standard ICalResource code.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/akonadi/kcal/kcalmimetypevisitor.h 998969 
>   /trunk/KDE/kdepim/akonadi/resources/ical/CMakeLists.txt 998969 
>   /trunk/KDE/kdepim/akonadi/resources/ical/Messages.sh 998969 
>   /trunk/KDE/kdepim/akonadi/resources/ical/icalresource.h 998969 
>   /trunk/KDE/kdepim/akonadi/resources/ical/icalresource.cpp 998969 
>   /trunk/KDE/kdepim/akonadi/resources/ical/icalresourcebase.h PRE-CREATION 
>   /trunk/KDE/kdepim/akonadi/resources/ical/icalresourcebase.cpp PRE-CREATION 
>   /trunk/KDE/kdepim/akonadi/resources/ical/icalresourcemain.cpp PRE-CREATION 
>   /trunk/KDE/kdepim/akonadi/resources/ical/notes/CMakeLists.txt PRE-CREATION 
>   /trunk/KDE/kdepim/akonadi/resources/ical/notes/notesmimetypevisitor.h PRE-CREATION 
>   /trunk/KDE/kdepim/akonadi/resources/ical/notes/notesmimetypevisitor.cpp PRE-CREATION 
>   /trunk/KDE/kdepim/akonadi/resources/ical/notes/notesresource.h PRE-CREATION 
>   /trunk/KDE/kdepim/akonadi/resources/ical/notes/notesresource.cpp PRE-CREATION 
>   /trunk/KDE/kdepim/akonadi/resources/ical/notes/notesresource.kcfg PRE-CREATION 
>   /trunk/KDE/kdepim/akonadi/resources/ical/notes/notesresourcemain.cpp PRE-CREATION 
>   /trunk/KDE/kdepim/akonadi/resources/ical/notes/settings.kcfgc PRE-CREATION 
>   /trunk/KDE/kdepim/akonadi/resources/ical/notesresource.desktop 998969 
> 
> Diff: http://reviewboard.kde.org/r/1057/diff
> 
> 
> Testing
> -------
> 
> Compiled, but not tested.
> 
> 
> Thanks,
> 
> David
> 
>

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