----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124331/#review82500 -----------------------------------------------------------
Ship it! src/kextracolumnsproxymodel.cpp (line 36) <https://git.reviewboard.kde.org/r/124331/#comment56910> the slots here is not required, no? only in the public part? if it's required, then you also need a 'public:' section below for the data members src/kextracolumnsproxymodel.cpp (line 44) <https://git.reviewboard.kde.org/r/124331/#comment56908> these two really should be QVector. As far as I can see, you do not interact with the Qt API and thus need QList for them, or am I missing something? QModelIndex is too large, and thus you'll allocate every item in the list on the heap. int is, on 64bit, too small and thus you waste space (factor of 2). src/kextracolumnsproxymodel.cpp (line 85) <https://git.reviewboard.kde.org/r/124331/#comment56909> here and elsewhere: why not use the newstyle connect syntax? - Milian Wolff On July 14, 2015, 10:42 a.m., David Faure wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124331/ > ----------------------------------------------------------- > > (Updated July 14, 2015, 10:42 a.m.) > > > Review request for KDE Frameworks, Jesper Pedersen and Stephen Kelly. > > > Repository: kitemmodels > > > Description > ------- > > REVIEW: 124331 > > > Diffs > ----- > > autotests/CMakeLists.txt f98e87d49b965998f31c5ede1fefb03b159a150f > autotests/kextracolumnsproxymodeltest.cpp PRE-CREATION > autotests/test_model_helpers.h a44db8a9cb985f69b52be96f0ffe389ebd903d6f > src/CMakeLists.txt 82d776f1fcc2f7b15e74f0ed47702ed570ce9892 > src/kextracolumnsproxymodel.h PRE-CREATION > src/kextracolumnsproxymodel.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/124331/diff/ > > > Testing > ------- > > Wrote autotests (as extensive as possible); used this in one real project, > AFAIK Jesper (blackie) did as well. > > > Thanks, > > David Faure > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
