Review Request: Proper avatar fetching in contact list

[Dario Freddi]

> On April 16, 2011, 7:27 a.m., David Edmundson wrote:
> > main-widget.cpp, line 809
> > <http://git.reviewboard.kde.org/r/101135/diff/1/?file=14482#file14482line809>
> >
> >     Should we pass the parent widget argument to KFileDialog::getImageOpenUrl so that the dialog loads next to the correct window.
> >     
> >     Also we should set a caption such as "Select an image to use an an avatar".
> >     
> >

True, I totally copied and pasted that part. Will fix that before pushing.


> On April 16, 2011, 7:27 a.m., David Edmundson wrote:
> > main-widget.cpp, line 833
> > <http://git.reviewboard.kde.org/r/101135/diff/1/?file=14482#file14482line833>
> >
> >     We should check (or Q_ASSERT) that the qobject_cast was successful before using fetchJob.

Q_ASSERT - that cast should always succeed.


> On April 16, 2011, 7:27 a.m., David Edmundson wrote:
> > main-widget.cpp, line 845
> > <http://git.reviewboard.kde.org/r/101135/diff/1/?file=14482#file14482line845>
> >
> >     I know this isn't your code, but should this be here at all?
> >     
> >     I would have expected userAccountIconButton to already have code to change upon the account->avatarChanged() signal, which means we don't need to do it ourselves here too.

I will check that and eventually submit a separate review request later today


- Dario


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


On April 15, 2011, 5:59 p.m., Dario Freddi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101135/
> -----------------------------------------------------------
> 
> (Updated April 15, 2011, 5:59 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> This patch implements proper avatar fetching in the contact list, leaving out a FIXME. The additional class is library-ready, as it needs to be used in other places than the CL. It uses KIO to be able to fetch (asynchronously) the file from every protocol KIO supports (so even http, ftp and stuff)
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 2b8506267a9966241d5dfb9ca2552afed11f7649 
>   fetch-avatar-job.h PRE-CREATION 
>   fetch-avatar-job.cpp PRE-CREATION 
>   main-widget.h 310dfb6864c3efdc27a97f587595f385b3b3b2a7 
>   main-widget.cpp 5fa5490ca950d2ffcaca18b1d56b7327cbc72906 
> 
> Diff: http://git.reviewboard.kde.org/r/101135/diff
> 
> 
> Testing
> -------
> 
> Needs testing, as I couldn't test myself!
> 
> 
> Thanks,
> 
> Dario
> 
>

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