Review Request: Merge request for telepathy-kde-contactlist

[David Edmundson]

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


This is so much easier to read than the first attempt, good work.

I'm finding it very hard to read a diff so large, so here are some first comments, though I might have more.


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

    I don't understand what this method is for.



main-widget.cpp
<http://git.reviewboard.kde.org/r/100838/#comment1786>

    If you need to manually invalidate your filter you have something wrong with either your model or the filter code.



main-widget.cpp
<http://git.reviewboard.kde.org/r/100838/#comment1767>

    I think the comment "fall Through" is in the wrong place.
    
    I initially read this as expecting ConnectionStatusDisconnected to fall through, which clearly it doesn't.



main-widget.cpp
<http://git.reviewboard.kde.org/r/100838/#comment1756>

    This is a bit confusing.
    
    1) do you really need two casts? why not qVariantValue<ContactModelItem*>(..)
    
    2) you don't need to model->data(index, role) you can just:
    
    index.data(role);
    


- David


On March 23, 2011, 9:32 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100838/
> -----------------------------------------------------------
> 
> (Updated March 23, 2011, 9:32 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> This is the work that begun in my clone repo and has matured enough now to be merged back to 'upstream', where the work should continue now.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 2c62ea10556cdba38d1bca0fe63603df97414336 
>   account-button.h PRE-CREATION 
>   account-button.cpp PRE-CREATION 
>   account-filter-model.h PRE-CREATION 
>   account-filter-model.cpp PRE-CREATION 
>   accounts-model-item.h PRE-CREATION 
>   accounts-model-item.cpp PRE-CREATION 
>   accounts-model.h PRE-CREATION 
>   accounts-model.cpp PRE-CREATION 
>   contact-delegate-overlay.h PRE-CREATION 
>   contact-delegate-overlay.cpp PRE-CREATION 
>   contact-delegate.h PRE-CREATION 
>   contact-delegate.cpp PRE-CREATION 
>   contact-model-item.h PRE-CREATION 
>   contact-model-item.cpp PRE-CREATION 
>   contact-overlays.h PRE-CREATION 
>   contact-overlays.cpp PRE-CREATION 
>   contact-view-hover-button.h PRE-CREATION 
>   contact-view-hover-button.cpp PRE-CREATION 
>   filter-bar.h PRE-CREATION 
>   filter-bar.cpp PRE-CREATION 
>   main-widget.h aed6f7c8336321bc8a6ffb3b9af6b1d493f85790 
>   main-widget.cpp 6146b62eec17b54be63b594200613931673ac5fe 
>   main-widget.ui 5b0d5aaaf3a4e2f49eb030b98b15fcae3a86170c 
>   main.cpp bba0e4175e8853afe603f26ea4707f4974192d0f 
>   ontologies/CMakeLists.txt 3e0bbe8c634908f689dcd360409aeddcc6fc0d23 
>   tree-node.h PRE-CREATION 
>   tree-node.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/100838/diff
> 
> 
> Testing
> -------
> 
> We did a lot of thourough testing on #kde-telepathy, also some people emailed their reports which all has been fixed.
> 
> 
> Thanks,
> 
> Martin
> 
>

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