[Kde-pim] Re: Review Request: Add support for disabled alarms in KCalCore

[Allen Winter]

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

Ship it!


looks fine. you may want to make static global objects, as shown in my comments


trunk/KDE/kdepimlibs/kcalcore/icalformat_p.cpp
<http://svn.reviewboard.kde.org/r/5475/#comment8131>

    static const char APP_NAME_FOR_XPROPERTIES[] = "KCALCORE";



trunk/KDE/kdepimlibs/kcalcore/icalformat_p.cpp
<http://svn.reviewboard.kde.org/r/5475/#comment8132>

    static const char ENABLED_ALARM_PROPERTY[] = "ENABLED";


- Allen


On 2010-09-29 15:14:59, Sergio Martins wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5475/
> -----------------------------------------------------------
> 
> (Updated 2010-09-29 15:14:59)
> 
> 
> Review request for KDE PIM, Allen Winter and Alvaro Manera.
> 
> 
> Summary
> -------
> 
> Incidence editors have a button called "Disable Alarm", which calls alarm->setEnabled( false ), but when the alarm is serialized to file, this information is lost. When the alarm is loaded again, it will be enabled.
> 
> Kolab's xml format has a property for supporting disabled alarms, but, because it's serialized to ical by akonadi, this info disappears.
> 
> This patch adds an KCalCore::Alarm custom property indicating if the alarm is enabled or not.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepimlibs/kcalcore/icalformat_p.cpp 1176540 
> 
> Diff: http://svn.reviewboard.kde.org/r/5475/diff
> 
> 
> Testing
> -------
> 
> Tested the editors. Unit tests are running.
> 
> 
> Thanks,
> 
> Sergio
> 
>

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