[Kde-pim] Review Request 111816: Server-side notification filtering

Volker Krause vkrause at kde.org
Wed Jul 31 18:03:06 BST 2013


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

Ship it!


Looks good, thanks! 

Is the use of QList in the D-Bus interface mandated by QtDBus? If not, QVector might be the slightly better choice there.

- Volker Krause


On July 31, 2013, 3:53 p.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111816/
> -----------------------------------------------------------
> 
> (Updated July 31, 2013, 3:53 p.m.)
> 
> 
> Review request for Akonadi and Volker Krause.
> 
> 
> Description
> -------
> 
> In the end, the change is quite small but I will feel more comfortable if someone shares the blame for breaking Akonadi again :-)
> 
> The most critical part is NotificationManager::findInterestedSources(). Currently clients are checking whether each notification matches filtering criteria of their Monitor, while this code does it the other way around - it finds all sources that accept (are interested in) each notification. This might be a bit faster, especially with huge amount of notifications. The logic should be almost identical to logic in Monitor::acceptNotification().
> 
> 
> Diffs
> -----
> 
>   interfaces/org.freedesktop.Akonadi.NotificationManager.xml fc53684 
>   interfaces/org.freedesktop.Akonadi.NotificationSource.xml 3cbea5a 
>   libs/notificationmessagev2.cpp 55d5708 
>   libs/notificationmessagev2_p.h f149f09 
>   server/src/notificationmanager.h caead9d 
>   server/src/notificationmanager.cpp f166c6b 
>   server/src/notificationsource.h f8270f1 
>   server/src/notificationsource.cpp 52273f6 
>   server/tests/unittest/CMakeLists.txt 8ca28f2 
>   server/tests/unittest/notificationmanagertest.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/111816/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

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


More information about the kde-pim mailing list