----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115394/#review48605 -----------------------------------------------------------
Looks good. Minor nitpicks: - I would use `#include <kcompletionbase.h>` instead of `#include "kcompletionbase.h"` for consistency - I would group the includes with the other kcompletion includes, except for kcompletionbase.cpp where it makes sense to include kcompletionbase.h first, to ensure kcompletionbase.h is self-contained. - Aurélien Gâteau On Jan. 30, 2014, 12:16 a.m., David Gil Oliva wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115394/ > ----------------------------------------------------------- > > (Updated Jan. 30, 2014, 12:16 a.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > ------- > > To make kcompletion.h smaller, split KCompletionBase class from it into its > own header file. > > > Diffs > ----- > > src/klineedit.h f1107e2 > src/kcompletionbase.cpp 252b613 > src/kcompletion.h fa0731e > src/kcompletionbase.h PRE-CREATION > src/CMakeLists.txt 05452ab > src/kcombobox.h f34d259 > > Diff: https://git.reviewboard.kde.org/r/115394/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
