----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121747/#review72758 -----------------------------------------------------------
+1 I like it. Some late-night nitpicking. Feel free to ignore as it's only in the unit test. autotests/kfiledialog_unittest.cpp <https://git.reviewboard.kde.org/r/121747/#comment50676> switch()? Use QStringLiteral? autotests/kfiledialog_unittest.cpp <https://git.reviewboard.kde.org/r/121747/#comment50679> Are you sure this builds? I thought QString(char* ) wasn't allowed in frameworks. - Aleix Pol Gonzalez On Dec. 29, 2014, 10:38 p.m., David Faure wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121747/ > ----------------------------------------------------------- > > (Updated Dec. 29, 2014, 10:38 p.m.) > > > Review request for KDE Frameworks, Lukáš Tinkl, Mark Gaiser, and Martin > Klapetek. > > > Repository: frameworkintegration > > > Description > ------- > > Commit 0b8ab82e7fa9ea0948a841ebf8523b62da made the view mode a choice of > the user (as it always was in KDE code) rather than a choice of the > developer (as QFileDialog::setViewMode allows). > > Typical Qt/KDE difference: Qt empowers the developer, KDE empowers the user. > I agree with Lukas that the latter makes more sense in KDE code. > > => I rewrote the unittest to check that the user's changes are restored > on the next file dialog. > > > Diffs > ----- > > autotests/kfiledialog_unittest.cpp e5108ead5a381864cfa2b8ddf62b5d6c64b13db9 > > Diff: https://git.reviewboard.kde.org/r/121747/diff/ > > > Testing > ------- > > > Thanks, > > David Faure > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
