Review Request 110227: [ktp-contact-list] Now playing shouldn't be disabled on pause

[David Edmundson]

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


Nicely done.


global-presence-chooser.cpp
<http://git.reviewboard.kde.org/r/110227/#comment23629>

    I think we can do
    
    lastPresenceIndex = m_model->rowCount();
    
    long term we need to come up with a way of making this all a bit neater, maybe a method in PresenceModelExtended that exposes the row type as an emum so we're not comparing magic index numbers, but that's not something we need to fix this.



global-presence-chooser.cpp
<http://git.reviewboard.kde.org/r/110227/#comment23628>

    Are you sure this >= is the right way round?
    
    I would have thought this should be "if we select a valid presence online/away/etc" which is if (index <= lastPresenceIndex) {
    
    }
    
    or am I getting confused?
    
    
    nitpick:
    
    coding style
    if () {
    
    }
    


- David Edmundson


On April 28, 2013, 12:55 a.m., Ian Monroe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110227/
> -----------------------------------------------------------
> 
> (Updated April 28, 2013, 12:55 a.m.)
> 
> 
> Review request for Telepathy and Martin Klapetek.
> 
> 
> Description
> -------
> 
> Now playing shouldn't be disabled on pause
> 
> Fix requires corresponding commit in ktp-kded-module https://git.reviewboard.kde.org/r/110228/
> Needed to separate things done for every presence change
> from those actions only to be done on user-initiated presence changes
> 
> 
> Diffs
> -----
> 
>   global-presence-chooser.h 678681c92afcbfcc85245b809bd29e84477eacfa 
>   global-presence-chooser.cpp f5d59399a904195121ebac7266bef4176ac82bfb 
> 
> Diff: http://git.reviewboard.kde.org/r/110227/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ian Monroe
> 
>

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