Review Request 113066: Refactor Now playing plugin

[David Edmundson]

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

Ship it!


Ship It!

- David Edmundson


On Oct. 2, 2013, 9:43 p.m., Xuetian Weng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113066/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 9:43 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Bugs: 307582 and 313126
>     http://bugs.kde.org/show_bug.cgi?id=307582
>     http://bugs.kde.org/show_bug.cgi?id=313126
> 
> 
> Repository: ktp-kded-module
> 
> 
> Description
> -------
> 
> This patch contains following changes:
> 1. disconnect for PropertiesChanges when knownPlayer is reset, the disappered service will not automatically disconnect from dbus.
> 2. change the "Enable" option to "Enable On Login" (bug 307582) and hence we can simplifies the logic (no more presenceActivated)
> 3. Changes now playing status at contact list will be remembered only for current login.
> 4. Fix the wrong behavior of handling PropertiesChanges (invalidate properties is not considered).
> 5. use requestedPresence to evaluate the current status (part of bug 313126)
> 6. since it changes to "Enable On Login", always enable the nowplaying text lineedit in kcm.
> 
> Current logic:
> 1. If the enabled option in config changes, set the plugin enable state (this must be caused by user change in kcm).
> 2. receive onActivate / onDeactivate from contact list, set the plugin enable state.
> 3. when enable state change to true, start detecting player. (activatePlugin(bool))
> 4. when enable state change to false, unwatch all player, reset all other state. (activatePlugin(bool))
> 5. when new player appears, request playbackStatus and metadata by GetAll, and then watch on the PropertiesChange for this player.
> 
> Some improvement:
> 1. not always listening on expensive serviceOwnerChanged.
> 2. be able to enable nowplaying on login.
> 3. fix leak connection of dbus signal.
> 4. code should now be clearer instead of having too much boolean check.
> 
> 
> Diffs
> -----
> 
>   config/telepathy-kded-config.cpp 953a637 
>   telepathy-mpris.h e3378d4 
>   telepathy-mpris.cpp 6cd3e20 
> 
> Diff: http://git.reviewboard.kde.org/r/113066/diff/
> 
> 
> Testing
> -------
> 
> Everything seems to work as expected.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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