Review Request 116940: State-affinity and engaging now Playing from a custom status message fixes.

David Edmundson david at davidedmundson.co.uk
Wed Mar 26 11:58:49 UTC 2014


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


Can you explain please.
"Fixes engage-nowPlaying-from-custom-status-message "

It really helps if I know what the bug we're trying to fix is and what's wrong with the existing code.


telepathy-module.h
<https://git.reviewboard.kde.org/r/116940/#comment37866>

    This is also in the struct above. 
    
    Storing the same data twice is almost always going to end in things going wrong.



telepathy-module.cpp
<https://git.reviewboard.kde.org/r/116940/#comment37867>

    We never write this. This is always going to be true.



telepathy-module.cpp
<https://git.reviewboard.kde.org/r/116940/#comment37868>

    I don't understand why we're splitting status messages and statuses.
    You're not saving disk writes/anything as the next if statement is going to trigger anyway as the statusMessage in the struct will differ.
    You're now actually doubling it. 



telepathy-module.cpp
<https://git.reviewboard.kde.org/r/116940/#comment37869>

    I don't understand this (and this is in the previous code)
    
    Why would we call setPresence when the requestedPresence has changed.   



telepathy-module.cpp
<https://git.reviewboard.kde.org/r/116940/#comment37865>

    +++
    
    All code in the kded should be requestedPresence. Well spotted, Thanks.


- David Edmundson


On March 26, 2014, 11:37 a.m., James Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116940/
> -----------------------------------------------------------
> 
> (Updated March 26, 2014, 11:37 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Bugs: 332082
>     http://bugs.kde.org/show_bug.cgi?id=332082
> 
> 
> Repository: ktp-kded-module
> 
> 
> Description
> -------
> 
> Fixes engage-nowPlaying-from-custom-status-message and lessens disk activity on presence change. Also simplifies logic used to keep telepathy-kded-module in a "fit" state during run-time.
> 
> 
> Diffs
> -----
> 
>   telepathy-module.cpp 030a0d9 
>   telepathy-module.h 2213cdf 
>   autoconnect.cpp dd36493 
>   autoconnect.h 9af58e7 
> 
> Diff: https://git.reviewboard.kde.org/r/116940/diff/
> 
> 
> Testing
> -------
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

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


More information about the KDE-Telepathy mailing list