Review Request: Merge request for telepathy-kde-contactlist

[David Edmundson]

> On March 25, 2011, 1:30 a.m., David Edmundson wrote:
> > main-widget.cpp, line 528
> > <http://git.reviewboard.kde.org/r/100838/diff/4/?file=12746#file12746line528>
> >
> >     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);
> >
> 
> Martin Klapetek wrote:
>     Actually I'm not sure if it is really needed (1), but since the model returns QVariant<QObject*>, I tried to directly cast it to ContactModelItem*, but this wouldn't compile for me. I'll try to play a little with it nevertheless.
> 
> David Edmundson wrote:
>     If it does require two casts (which you may well be right about) at least use qobject_cast between the QObject* and ContactModelItem*. It has an extra check in which makes it safer.
>
> 
> Dario Freddi wrote:
>     This is actually a misuse of QVariant. The error you got when compiling was probably due to the fact that ContactModelItem* has not been declared as a MetaType through the Q_DECLARE_METATYPE(ContactModelItem*) macro (http://doc.qt.nokia.com/4.7/qmetatype.html#Q_DECLARE_METATYPE), hence Qt can not recognize it as a type QVariant can hold. The right thing to do is to declare the metatype correctly (you might want to do that at the bottom of ContactModelItem's header), and then use QVariant::value() to extract the type without resorting to hacks. The code you should obtain should look like:
>     
>     Tp::ContactPtr contact = m_model->data(realIndex, AccountsModel::ItemRole).value<ContactModelItem*>()->contact();
>     
>     Please check if you had to resort to similar hacks in other portions of the code, and fix them accordingly.

Done.


- David


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


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/20110401/96f9a72a/attachment.htm