----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109500/#review29557 -----------------------------------------------------------
plasmate/savesystem/dvcsjob.h <http://git.reviewboard.kde.org/r/109500/#comment22039> is this used anywhere? also, it is tradition to put the "Unknown" defintion as the first item in the enumeration, set to 0. plasmate/savesystem/gitrunner.cpp <http://git.reviewboard.kde.org/r/109500/#comment22040> const QString? plasmate/savesystem/gitrunner.cpp <http://git.reviewboard.kde.org/r/109500/#comment22041> i wonder if this works if the shell is set to a different lang environment? or is git simply not translated? plasmate/savesystem/gitrunner.cpp <http://git.reviewboard.kde.org/r/109500/#comment22042> for code style purposes, we treat foreach as a flow control operator like while and for, not as a function. so it should be foreach ( plasmate/savesystem/gitrunner.cpp <http://git.reviewboard.kde.org/r/109500/#comment22043> perhaps startsWith would be better? or if there are possible spaces before the *, do the space replacement below first. (yes, * is not a valid character for git branch names currently .. but i wouldn't rely on that forever and startsWith is faster than contains) plasmate/savesystem/timeline.cpp <http://git.reviewboard.kde.org/r/109500/#comment22044> while ( plasmate/savesystem/timeline.cpp <http://git.reviewboard.kde.org/r/109500/#comment22045> const QString? plasmate/savesystem/timeline.cpp <http://git.reviewboard.kde.org/r/109500/#comment22046> const QString? plasmate/savesystem/timeline.cpp <http://git.reviewboard.kde.org/r/109500/#comment22047> const QString? plasmate/savesystem/timeline.cpp <http://git.reviewboard.kde.org/r/109500/#comment22048> you must always check the return value of qobject_cast (it's just like dynamic_cast) so in this case: QAction *sender = qobject_cast<QAction*>(this->sender()); if (!sender) { return QString(); } - Aaron J. Seigo On March 15, 2013, 4:52 p.m., Giorgos Tsiapaliokas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109500/ > ----------------------------------------------------------- > > (Updated March 15, 2013, 4:52 p.m.) > > > Review request for Plasma. > > > Description > ------- > > Hello, > > this patch is the refactor of the savesystem. > > Also in this refactor the above bugs are being fixed. > > Q: Why do we need a refactor? > A: * Before this patch in order to take the git log we did > some parsing, which wasn't nice. With this patch we use git log > --pretty-format. > * There are some other changes like coding style stuff and striping \n from > strings. > > > This addresses bugs 316202, 316724 and 316725. > http://bugs.kde.org/show_bug.cgi?id=316202 > http://bugs.kde.org/show_bug.cgi?id=316724 > http://bugs.kde.org/show_bug.cgi?id=316725 > > > Diffs > ----- > > plasmate/savesystem/dvcsjob.h 38df371 > plasmate/savesystem/dvcsjob.cpp 288d7a6 > plasmate/savesystem/gitrunner.h 50f87ab > plasmate/savesystem/gitrunner.cpp b263be6 > plasmate/savesystem/timeline.h 73849d0 > plasmate/savesystem/timeline.cpp 231fc46 > > Diff: http://git.reviewboard.kde.org/r/109500/diff/ > > > Testing > ------- > > > Thanks, > > Giorgos Tsiapaliokas > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
