Review Request: Add group online/total users count

[David Edmundson]

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


Mostly looks pretty sensible, got a few minor comments.


account-filter-model.cpp
<http://git.reviewboard.kde.org/r/101559/#comment3102>

    Why this?
    
    You've added a role for getting that info.
    AccountsModel::OnlineUsersCountRole:
    
    Related: I don't think groupsmodelitem.h needs to be in the filter at all.



accounts-model-item.cpp
<http://git.reviewboard.kde.org/r/101559/#comment3101>

    check casts with Q_ASSERT.



accounts-model-item.cpp
<http://git.reviewboard.kde.org/r/101559/#comment3097>

    This isn't the same as used in the filter. 
    
    The filter also checks for Unknown. 
    
    They should be consistent. (which could be removing it from the filter) Otherwise we could potentially see (5/6)  only show 4 items.



groups-model-item.cpp
<http://git.reviewboard.kde.org/r/101559/#comment3099>

    hate to write this one but:
    
    TpQt4 coding style seems to be
    
    mSomethingSomething instead of our
    
    m_somethingSomething. 
    
    We should stick to the TpQt4 style in this class (if we want merging)



groups-model-item.cpp
<http://git.reviewboard.kde.org/r/101559/#comment3098>

    instead of toUint can you do
    
    .value<ConntectionPresenecType>() ?
    
    it's safer to treat enums like enums if possible.
    



groups-model.cpp
<http://git.reviewboard.kde.org/r/101559/#comment3100>

    Always check your casts.
    
    Q_ASSERT.
     


- David


On June 9, 2011, 9:15 a.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101559/
> -----------------------------------------------------------
> 
> (Updated June 9, 2011, 9:15 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> This patch adds numbers to group headers in "(online users/total users in group)" scheme. Also it allows to filter out empty groups.
> 
> 
> Diffs
> -----
> 
>   abstract-contact-delegate.cpp db2f3de 
>   account-filter-model.cpp 2542ae6 
>   accounts-model-item.h 54d8f87 
>   accounts-model-item.cpp 6facdea 
>   accounts-model.h 96e8994 
>   accounts-model.cpp 47fc697 
>   groups-model-item.h 63f3973 
>   groups-model-item.cpp 6eec952 
>   groups-model.cpp 46631e4 
>   main-widget.cpp 7c65760 
> 
> Diff: http://git.reviewboard.kde.org/r/101559/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110609/d939e367/attachment.htm