> On 2010-03-29 11:08:34, Sebastian Kügler wrote: > >
Thanks for your review. Next time i'll try to do better. (Allredy in progress) > On 2010-03-29 11:08:34, Sebastian Kügler wrote: > > trunk/KDE/kdeplasma-addons/applets/frame/configdialog.h, line 77 > > <http://reviewboard.kde.org/r/3334/diff/1/?file=21279#file21279line77> > > > > Maybe reloadAutomatically() or toggleAutoReload(), the function name > > isn't really clear Yes, i'm pretty bad in giving names. In the next diff, i'll follow Aarons advice and therefor this function isn't needed anymore. > On 2010-03-29 11:08:34, Sebastian Kügler wrote: > > trunk/KDE/kdeplasma-addons/applets/frame/configdialog.h, line 52 > > <http://reviewboard.kde.org/r/3334/diff/1/?file=21279#file21279line52> > > > > trailing spaces, please get rid of those Ok;) > On 2010-03-29 11:08:34, Sebastian Kügler wrote: > > trunk/KDE/kdeplasma-addons/applets/frame/configdialog.cpp, line 160 > > <http://reviewboard.kde.org/r/3334/diff/1/?file=21280#file21280line160> > > > > the livecam naming is a bit confusing, maybe put what it does? > > Something with automatic reloading? > > > > no const needed here afaik i changed that. > On 2010-03-29 11:08:34, Sebastian Kügler wrote: > > trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp, line 116 > > <http://reviewboard.kde.org/r/3334/diff/1/?file=21282#file21282line116> > > > > "" -> QString() (also elsewhere) thanks, didn't know that. > On 2010-03-29 11:08:34, Sebastian Kügler wrote: > > trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp, line 119 > > <http://reviewboard.kde.org/r/3334/diff/1/?file=21282#file21282line119> > > > > Use the one from KGlobal here as default that makes more sense will be kicket out in the next diff. > On 2010-03-29 11:08:34, Sebastian Kügler wrote: > > trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp, line 452 > > <http://reviewboard.kde.org/r/3334/diff/1/?file=21282#file21282line452> > > > > kDebug(), English please :) Yeah. forgot those... > On 2010-03-29 11:08:34, Sebastian Kügler wrote: > > trunk/KDE/kdeplasma-addons/applets/frame/imageSettings.ui, line 160 > > <http://reviewboard.kde.org/r/3334/diff/1/?file=21283#file21283line160> > > > > hh:mm should be good enough, this way the text would look less confusing Good point. I'll change that. - eti ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3334/#review4733 ----------------------------------------------------------- On 2010-03-27 15:08:02, eti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/3334/ > ----------------------------------------------------------- > > (Updated 2010-03-27 15:08:02) > > > Review request for Plasma. > > > Summary > ------- > > I was quite happy that the picture frame could handle url's in 4.4. > So i added the ability to update the picture with an url periodicaly (i.e. > live cams, weather data or <picture that changes over time>). > This would be a rather small fix, but i also came accross Bug 222759 and with > the updatefunction the download path got quite spamed. > Now it's possible to set the download path and the ability to ovewrite the > existing picture when downloaded. > > By the way, i also removed the picture class from the slideshow class. The > slideshow now manages only the path's and the frame class acceses the picture > class directly. > > Let me know wat you think;) > > > Diffs > ----- > > trunk/KDE/kdeplasma-addons/applets/frame/CMakeLists.txt 1105894 > trunk/KDE/kdeplasma-addons/applets/frame/configdialog.h 1105894 > trunk/KDE/kdeplasma-addons/applets/frame/configdialog.cpp 1105894 > trunk/KDE/kdeplasma-addons/applets/frame/frame.h 1105894 > trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp 1105894 > trunk/KDE/kdeplasma-addons/applets/frame/imageSettings.ui 1105894 > trunk/KDE/kdeplasma-addons/applets/frame/picture.h 1105894 > trunk/KDE/kdeplasma-addons/applets/frame/picture.cpp 1105894 > trunk/KDE/kdeplasma-addons/applets/frame/slideshow.h 1105894 > trunk/KDE/kdeplasma-addons/applets/frame/slideshow.cpp 1105894 > trunk/KDE/kdeplasma-addons/applets/frame/urlSettings.ui PRE-CREATION > > Diff: http://reviewboard.kde.org/r/3334/diff > > > Testing > ------- > > > Thanks, > > eti > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
