Review Request: Merge request for telepathy-kde-contactlist

Dario Freddi drf at kde.org
Thu Mar 31 15:36:45 CEST 2011



> On March 31, 2011, 12:03 p.m., David Edmundson wrote:
> > main-widget.cpp, line 136
> > <http://git.reviewboard.kde.org/r/100838/diff/4/?file=12746#file12746line136>
> >
> >     This doesn't make sense to me, you're filling in people's name and avatar at the top of the contact list, but not actually updating telepathy.
> >     
> >     People will get confused if you're showing them things which are different to what other contacts viewing the list will see.
> >     
> >     I think I want these two lines deleted unless you can explain the logic here.
> >     
> >     (also you can just user.fullname() )
> 
> Martin Klapetek wrote:
>     The idea to use system avatar was because of multiple accounts, which can all have different avatar. So the solution here is to either take system avatar and set it for all Tp accounts, but we need to be careful here as most people don't have the avatar set, or we can take avatar of first added account and use it (and apply it to the rest of the accounts). Or we can let the user choose from which account he wants to use the avatar. I would go for the second idea for now as that would be the easiest. Also, we could then set this avatar as the system one, what do you think?
> 
> David Edmundson wrote:
>     I think I want this merged whole branch merged as soon as possible (being stuck in review is really slowing it down for any other people to commit here) and that this whole discussion should take place afterwards as I don't think there's a single simple solution.
>     
>     I would go with commenting this out for now as the least controversial. Then ask the ML how to go about this.

Besides David's comment, you are also doing a lot of unneeded conversions here (slipped in my previous review). You could simply do:

QIcon icon;
icon.addFile(user.faceIconPath());
m_userAccountIconButton->setIcon(icon);


- Dario


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


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/20110331/6f3f931b/attachment.htm 


More information about the KDE-Telepathy mailing list