----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113298/#review41860 -----------------------------------------------------------
There are couple issues still, I pointed only bunch of them as they repeat themselves, so you might want to keep an eye on those Also, kdelibs coding style - http://techbase.kde.org/Policies/Kdelibs_Coding_Style tier1/kcoreaddons/src/lib/io/kdirwatch.h <http://git.reviewboard.kde.org/r/113298/#comment30546> As per the kdelibs coding style, there should be spaces around operators too, so eg. "notify = false" etc tier1/kcoreaddons/src/lib/io/kdirwatch.h <http://git.reviewboard.kde.org/r/113298/#comment30547> The & sign should be aligned at the variable name, eg. "QString &path" (like you have below) tier1/kcoreaddons/src/lib/io/kdirwatch.cpp <http://git.reviewboard.kde.org/r/113298/#comment30548> Entry* e -> Entry *e tier1/kcoreaddons/src/lib/io/kdirwatch.cpp <http://git.reviewboard.kde.org/r/113298/#comment30549> There should be no spaces before ";", eg. "...!= end; ++it" tier1/kcoreaddons/src/lib/io/kdirwatch.cpp <http://git.reviewboard.kde.org/r/113298/#comment30550> Indent on 4 spaces tier1/kcoreaddons/src/lib/io/kdirwatch.cpp <http://git.reviewboard.kde.org/r/113298/#comment30551> Operator spaces: if (res < 0) { tier1/kcoreaddons/src/lib/io/kdirwatch.cpp <http://git.reviewboard.kde.org/r/113298/#comment30552> Operator spaces as well tier1/kcoreaddons/src/lib/io/kdirwatch.cpp <http://git.reviewboard.kde.org/r/113298/#comment30553> This should probably be on separate lines tier1/kcoreaddons/src/lib/io/kdirwatch.cpp <http://git.reviewboard.kde.org/r/113298/#comment30554> Same as the block above tier1/kcoreaddons/src/lib/io/kdirwatch.cpp <http://git.reviewboard.kde.org/r/113298/#comment30555> Entry *e tier1/kcoreaddons/src/lib/io/kdirwatch.cpp <http://git.reviewboard.kde.org/r/113298/#comment30556> Indent one level more (should be 8 spaces at this level)....actually this whole file seems to have wrong indentation, so maybe fix all of that? tier1/kcoreaddons/src/lib/io/kdirwatch.cpp <http://git.reviewboard.kde.org/r/113298/#comment30557> Pointer signs to the variable -> KDirWatch *instance etc tier1/kcoreaddons/src/lib/io/kdirwatch.cpp <http://git.reviewboard.kde.org/r/113298/#comment30558> Should be at the "if" line, eg. "if () {" tier1/kcoreaddons/src/lib/io/kdirwatch_p.h <http://git.reviewboard.kde.org/r/113298/#comment30559> Spaces inside -> "KDirWatch *, bool" - Martin Klapetek On Oct. 17, 2013, 3:51 a.m., Nicolás Alvarez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113298/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2013, 3:51 a.m.) > > > Review request for KDE Frameworks and kdelibs. > > > Repository: kdelibs > > > Description > ------- > > KDirWatch code style: cleanup whitespace. > > The KDirWatch code had *lots* of "( foo )" and inconsistent indentation and > alignment, including a few tabs(!). This is a full cleanup of it. > > I appreciate any feedback; if I "fixed" something that didn't need fixing, or > if you see more whitespace errors that I didn't fix, or if I should push this > to master too, or if I should leave the damn thing alone and discard the > review :) > > This file is also lacking braces for single-line conditionals and loops; I'll > fix that in a separate commit for easier reviewing. It's also mixing 2-space > and 4-space indentations, but changing everything to 4 spaces (as the kdelibs > coding style says) seemed too intrusive. Perhaps I should change the few > 4-space indentations into 2-space for consistency? > > > Diffs > ----- > > tier1/kcoreaddons/src/lib/io/kdirwatch.h > 7f6ca8ce83426c81a6336514c247aa9d115ec59e > tier1/kcoreaddons/src/lib/io/kdirwatch.cpp > e4f45441d5ed68e3e34ae2bd68e16fd3dc46656a > tier1/kcoreaddons/src/lib/io/kdirwatch_p.h > 442d6497b704c179adc13dbb25e450554d31554d > > Diff: http://git.reviewboard.kde.org/r/113298/diff/ > > > Testing > ------- > > Still compiles :) > > > Thanks, > > Nicolás Alvarez > >
