Review Request 116940: Add a new status message plugin class and use it for the now Playing plugin

[James Smith]

> On Aug. 8, 2014, 12:11 p.m., David Edmundson wrote:
> > telepathy-kded-module-message-plugin.cpp, line 46
> > <https://git.reviewboard.kde.org/r/116940/diff/20/?file=300642#file300642line46>
> >
> >     This does nothing.
> >     
> >     you connect in the ctor, and don't disconnect anywhere.

This makes sure that I don't trigger !enable with a pending activate(). This makes certain that the plugins are allowed time to clean up their output before they're turned off.


> On Aug. 8, 2014, 12:11 p.m., David Edmundson wrote:
> > status-handler.cpp, line 109
> > <https://git.reviewboard.kde.org/r/116940/diff/20/?file=300640#file300640line109>
> >
> >     we don't need to guard round making sure we don't do pointless savePresence calls; kconfig will do that for us.

KConfig won't guard against plugin-set status messages being saved as custom user-set messages, and also doesn't distinguish between the two.


> On Aug. 8, 2014, 12:11 p.m., David Edmundson wrote:
> > status-handler.cpp, line 183
> > <https://git.reviewboard.kde.org/r/116940/diff/20/?file=300640#file300640line183>
> >
> >     I don't get why we're doing this.
> >     
> >     if the plugin is active and it's not the message we currently have, turn the plugin off?
> >     
> >     That's why we have the queue?

This keeps all the status message plugins that aren't actually providing a status message disabled. This behaviour might impact the usefulness of the queue to allow multiple plugins, in the case of the active plugin turning off, to have the next prioritied plugin take over.


- James


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116940/#review64058
-----------------------------------------------------------


On Aug. 7, 2014, 12:17 a.m., James Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116940/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2014, 12:17 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Bugs: 332082, 334492 and 334542
>     http://bugs.kde.org/show_bug.cgi?id=332082
>     http://bugs.kde.org/show_bug.cgi?id=334492
>     http://bugs.kde.org/show_bug.cgi?id=334542
> 
> 
> Repository: ktp-kded-module
> 
> 
> Description
> -------
> 
> Move status message plugins into a separate class. New infrastructure to help mitigate the occurrence of bug #334492 while using status message plugins: 1) buffering of the last n status messages, and 2) a limiter to help stop flooding the presence changer. Also strips redundant and disused data structures from both classes.
> 
> This patch returns the ability to engage status message plug-ins from custom status messages. Also working is the disabling of non-visible status message plug-ins. State-affinity in the 95% of previously noted cases has been vastly improved also, the few remaining issues should be due to "lite" protocols that don't have a full complement of on-line presences.
> 
> 
> Diffs
> -----
> 
>   telepathy-kded-module-message-plugin.cpp PRE-CREATION 
>   CMakeLists.txt 930267740d0bf26f42a48ea55d77148f87df5369 
>   status-handler.h 06240ff17e22148f2b128bc0eb8cec6d6abe68ff 
>   status-handler.cpp 4b9c25a2ccba451f6e608bb704626e33149108cc 
>   telepathy-kded-module-message-plugin.h PRE-CREATION 
>   telepathy-kded-module-plugin.h 4c161696a706e82059a7eb314773c3644fe26bd7 
>   telepathy-mpris.cpp 44b041fdd3764ee5f67598fcf555a2759d853bdd 
>   tests/CMakeLists.txt 7ec77495417a6790060ea5ea7126c46399dff755 
>   telepathy-kded-module-plugin.cpp daf73c66947bc946097de7a8e8a1518555131145 
>   telepathy-mpris.h 05b77c90a50372fd9ed66bde0ab8a287caf34b51 
> 
> Diff: https://git.reviewboard.kde.org/r/116940/diff/
> 
> 
> Testing
> -------
> 
> Disconnect / reconnect, autoconnect / no autoconnect, suspend / resume. Enable / disable via kcm module. Added a new custom presence and engaged the now playing plugin in the contact list from the new presence. Disabled the plugin by activating another presence.
> 
> 
> Thanks,
> 
> James Smith
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140808/375c89af/attachment-0001.html>