----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3334/#review4733 -----------------------------------------------------------
trunk/KDE/kdeplasma-addons/applets/frame/configdialog.h <http://reviewboard.kde.org/r/3334/#comment4233> trailing spaces, please get rid of those trunk/KDE/kdeplasma-addons/applets/frame/configdialog.h <http://reviewboard.kde.org/r/3334/#comment4235> Maybe reloadAutomatically() or toggleAutoReload(), the function name isn't really clear trunk/KDE/kdeplasma-addons/applets/frame/configdialog.cpp <http://reviewboard.kde.org/r/3334/#comment4234> remove spacs (... and trailing trunk/KDE/kdeplasma-addons/applets/frame/configdialog.cpp <http://reviewboard.kde.org/r/3334/#comment4236> the livecam naming is a bit confusing, maybe put what it does? Something with automatic reloading? no const needed here afaik trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp <http://reviewboard.kde.org/r/3334/#comment4238> "" -> QString() (also elsewhere) trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp <http://reviewboard.kde.org/r/3334/#comment4237> Use the one from KGlobal here as default that makes more sense trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp <http://reviewboard.kde.org/r/3334/#comment4239> use kDebug trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp <http://reviewboard.kde.org/r/3334/#comment4240> kDebug() trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp <http://reviewboard.kde.org/r/3334/#comment4241> kDebug(), English please :) trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp <http://reviewboard.kde.org/r/3334/#comment4242> You're creating a magic value here (""), why not use the KGlobal downloadpath? Also, this statement should read !m_downloadPath.isEmpty() trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp <http://reviewboard.kde.org/r/3334/#comment4243> space between ) and { trunk/KDE/kdeplasma-addons/applets/frame/imageSettings.ui <http://reviewboard.kde.org/r/3334/#comment4244> Useful (single l), came -> cam trunk/KDE/kdeplasma-addons/applets/frame/imageSettings.ui <http://reviewboard.kde.org/r/3334/#comment4245> hh:mm should be good enough, this way the text would look less confusing trunk/KDE/kdeplasma-addons/applets/frame/picture.h <http://reviewboard.kde.org/r/3334/#comment4246> trailing space trunk/KDE/kdeplasma-addons/applets/frame/picture.cpp <http://reviewboard.kde.org/r/3334/#comment4247> 4 spaces indenting trunk/KDE/kdeplasma-addons/applets/frame/urlSettings.ui <http://reviewboard.kde.org/r/3334/#comment4248> get -> gets trunk/KDE/kdeplasma-addons/applets/frame/urlSettings.ui <http://reviewboard.kde.org/r/3334/#comment4249> overwritten - Sebastian 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
