[Kde-pim] Review Request: Maemo fixes in Kcal

[Alvaro Manera]

> On 2009-10-15 14:45:03, Allen Winter wrote:
> > there are some minor problems (like forgetting the @since 4.4 apidox tags)
> > which I fixed locally.
> > 
> > there is a major problem of binary incompatibility by adding the incidenceUpdate() pure virtual.
> > Not quite sure how to deal with 
> > the other questions is: would you be willing to use Person objects with the new Contacts methods instead of QStrings?
> 
> Alvaro Manera wrote:
>     And what is your proposal to get rid of the binary incompatibility? Because I don't want to wait to kde5 to get it in. :)
> 
> Kevin Krammer wrote:
>     One option is to create an IncidenceObserverV2 which inherits IncidenceObserver and has this additional method.
>     Then either registerObserver() or update() need to decide which of one of them is the base for the observer at hand.
>
> 
> Allen Winter wrote:
>     Alvaro, I think we should trying using signals and slots as described in http://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++#Adding_new_virtual_functions_to_leaf_classes "Using signals instead of virtual functions".  
>     
>     Would you want to investigate doing that? 
>     
>     Else, we can commit Contacts stuff now only and also we need to deal with the recurrenceID stuff.

We cannot follow the signal slot approach. The new virtual is in the IncidenceObserver class. We need to add the inheritance to QObject. And even if we could, Calendar class inherits from it (and also from QObject). We will need to change the inheritance order again. 
This way we cannot fix the ABI break.
The other option is to create the V2 class. And add the additional method. But this is valid for me, KDE will have the fix in code, but no possibility of use it. Because to do it, the inheritance change in Calendar will be needed.
Any other ideas?


- Alvaro


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


On 2009-10-21 14:48:51, Alvaro Manera wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1489/
> -----------------------------------------------------------
> 
> (Updated 2009-10-21 14:48:51)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> This is a combined patch of all the changes we have in our trunk, before the implementation of the RecurrenceID.
> 
> These changes include:
> * Contacts in the incidence. (I don't know if you need this, as you have the addressbook) ¿?
> * Some small fixes in our side (like copy constructor in listbase).
> * Handling the updates. This fixes the bug that you end up having a duplicate entry in the Hash table when modifying an event. 
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepimlibs/kcal/alarm.cpp 1035467 
>   trunk/KDE/kdepimlibs/kcal/calendarlocal.h 1035467 
>   trunk/KDE/kdepimlibs/kcal/calendarlocal.cpp 1035467 
>   trunk/KDE/kdepimlibs/kcal/calendarnull.h 1035467 
>   trunk/KDE/kdepimlibs/kcal/calendarnull.cpp 1035467 
>   trunk/KDE/kdepimlibs/kcal/event.cpp 1035467 
>   trunk/KDE/kdepimlibs/kcal/icalformat_p.cpp 1035467 
>   trunk/KDE/kdepimlibs/kcal/incidence.cpp 1035467 
>   trunk/KDE/kdepimlibs/kcal/incidencebase.h 1035467 
>   trunk/KDE/kdepimlibs/kcal/incidencebase.cpp 1035467 
>   trunk/KDE/kdepimlibs/kcal/journal.cpp 1035467 
>   trunk/KDE/kdepimlibs/kcal/listbase.h 1035467 
>   trunk/KDE/kdepimlibs/kcal/todo.cpp 1035467 
> 
> Diff: http://reviewboard.kde.org/r/1489/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alvaro
> 
>

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