> On April 22, 2014, 9:50 a.m., Kevin Krammer wrote: > > src/lib/util/kdelibs4migration.cpp, line 81 > > <https://git.reviewboard.kde.org/r/117511/diff/2/?file=267469#file267469line81> > > > > would QStringLiteral work here?
Given the multiple implementations of QStringLiteral depending on compiler and C++11 support, I'd rather not risk it. IIRC it uses a lambda on msvc..... sounds tricky. > On April 22, 2014, 9:50 a.m., Kevin Krammer wrote: > > src/lib/util/kdelibs4migration.cpp, line 82 > > <https://git.reviewboard.kde.org/r/117511/diff/2/?file=267469#file267469line82> > > > > Hmm. I think that looks weird. > > Can this be split in the type definition (struct something) and the > > constant defintion? I don't get that. If I make a list of type names, what do I do with it? All I need is a type->subdir lookup table (just like kstandarddirs has/had, btw) > On April 22, 2014, 9:50 a.m., Kevin Krammer wrote: > > src/lib/util/kdelibs4migration.cpp, line 97 > > <https://git.reviewboard.kde.org/r/117511/diff/2/?file=267469#file267469line97> > > > > Also maybe just a personal taste, but I find it better to explicitly > > use parentheses when mixing boolean and arithmetic operators, i.e. make it > > explicit which operator has precendence. In this case putting parentheses > > around the size calculation. > > Or even calculating the size as a const int before the loop (can it be > > done as a const_expr outside the function?). > > > > Kevin Krammer wrote: > Or as a std:find_if()? > Sorry, have just watched a Going Native talk about "no raw loops" :) std::find_if requires iterators, which I don't really have here. Plus it's not usual in KDE code (so it reduces maintainability/readability). I moved the size as a const int before the loop (which actually improves readability). > On April 22, 2014, 9:50 a.m., Kevin Krammer wrote: > > src/lib/util/kdelibs4migration.cpp, line 106 > > <https://git.reviewboard.kde.org/r/117511/diff/2/?file=267469#file267469line106> > > > > Do we have some logging categories for kdecoreaddons? No. Porting to categorized logging has to be done everywhere in frameworks.... - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117511/#review56172 ----------------------------------------------------------- On April 22, 2014, 9:32 a.m., David Faure wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/117511/ > ----------------------------------------------------------- > > (Updated April 22, 2014, 9:32 a.m.) > > > Review request for KDE Frameworks, Ivan Čukić and Kevin Krammer. > > > Repository: kcoreaddons > > > Description > ------- > > Add class for finding the kde4 config and apps home dirs. > > To help applications migrating to the kf5/qt5 directories. > > > Diffs > ----- > > autotests/CMakeLists.txt 2f14b3a229b07071ed6e8b0772e03ee798db6c03 > autotests/kdelibs4migrationtest.cpp PRE-CREATION > src/lib/CMakeLists.txt 39ca3b8e9d5a4f8ffa06ca2ccf017b02ac245fd7 > src/lib/util/kdelibs4migration.h PRE-CREATION > src/lib/util/kdelibs4migration.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/117511/diff/ > > > Testing > ------- > > > Thanks, > > David Faure > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel