> 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
> 
>

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to