----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115419/#review48900 -----------------------------------------------------------
It's getting there :-) src/kcompletion.cpp <https://git.reviewboard.kde.org/r/115419/#comment34526> Not needed. src/kcompletion_p.h <https://git.reviewboard.kde.org/r/115419/#comment34528> This is duplicated across two files (this one and kcompletionmatches.h), which is less than ideal. What about adding #include <kcompletionmatches.h> to this file? src/kcompletion_p.h <https://git.reviewboard.kde.org/r/115419/#comment34527> Why is this removed? src/kcompletionmatches_p.h <https://git.reviewboard.kde.org/r/115419/#comment34525> This class is not used outside kcompletionmatches.cpp, so you don't need it (and the class definition is far too short to be worth having a separate header). Put this at the top of kcompletionmatches.cpp, and discard this file. - Alex Merry On Feb. 4, 2014, 12:36 a.m., David Gil Oliva wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115419/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2014, 12:36 a.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > ------- > > To make code clearer, split KCompletionMatches. > > > Diffs > ----- > > src/CMakeLists.txt ae4a656 > src/kcompletion.cpp c684727 > src/kcompletion.h f197fc3 > src/kcompletionmatches_p.h PRE-CREATION > src/kcompletionmatches.cpp PRE-CREATION > src/kcompletionmatches.h PRE-CREATION > src/kcompletion_p.h a8dedae > > Diff: https://git.reviewboard.kde.org/r/115419/diff/ > > > Testing > ------- > > It builds. Tests pass. > > > Thanks, > > David Gil Oliva > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel