> 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 > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
