Review Request: KDED Module refactor

David Edmundson kde at davidedmundson.co.uk
Sat Oct 8 11:16:46 UTC 2011


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



telepathy-kded-module-plugin.h
<http://git.reviewboard.kde.org/r/102804/#comment6264>

    make this bool isActive() const
    
    same for other getters



telepathy-kded-module-plugin.h
<http://git.reviewboard.kde.org/r/102804/#comment6265>

    It's nicer practice to have members private, and have protected accessors/setters. 
    
    Not really worth it - except maybe for "active". That way your PluginClass can ensure the signal is emitted correctly.



telepathy-kded-module-plugin.h
<http://git.reviewboard.kde.org/r/102804/#comment6266>

    This is a singleton anyway?


- David Edmundson


On Oct. 8, 2011, 11:15 a.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102804/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2011, 11:15 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Quite a big rewrite of the KDED module. The main changes are:
> 
> 1/ Use stacking system for plugins
>    --each plugin has its own priority, if the priority is higher than what's currently in stack, the plugin is placed on top, if it's lower, it's placed below (maintaining the order by priorities)
> 
> 2/ Plugins now use this sort-of API
>   --when any plugin wants to set a presence, it signals active(true); which is connected to the module's core where it checks the stack and either uses that plugin's presence or places it somewhere down the stack
> 
> 3/ Plugins all have common base class
> 
> 4/ 'Now listening...' aka mpris plugin was gifted with some refactoring as well
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 143afef 
>   autoaway.h a95f49a 
>   autoaway.cpp 812ba97 
>   global-presence.h PRE-CREATION 
>   global-presence.cpp PRE-CREATION 
>   telepathy-kded-module-plugin.h PRE-CREATION 
>   telepathy-kded-module-plugin.cpp PRE-CREATION 
>   telepathy-module.h 815ee38 
>   telepathy-module.cpp 174cecf 
>   telepathy-mpris.h 5dd596f 
>   telepathy-mpris.cpp b4920d8 
> 
> Diff: http://git.reviewboard.kde.org/r/102804/diff/diff
> 
> 
> Testing
> -------
> 
> 1/ Works
> 2/ Works
> 3/ Works too, obviously
> 4/ Still not with configurable message, but works
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20111008/c05c0ef8/attachment.html>


More information about the KDE-Telepathy mailing list