Review Request 115419: Split KCompletionMatches class into files of its own
Alex Merry
kde at randomguy3.me.uk
Tue Feb 4 23:45:52 UTC 2014
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115419/#review48988
-----------------------------------------------------------
Ship it!
Hmm... I feel that KCompletionMatchesWrapper is in the wrong place, but I'm not entirely sure where it *should* go. Well, it's not in a public header, so that's far less urgent (although a potential later task would be to move the private methods of KCompletion, some of which take KCompletionMatchesWrapper as an argument, to KCompletionMatchesPrivate).
Anyway, resolve the last two issues, and it can go in.
src/kcompletion_p.h
<https://git.reviewboard.kde.org/r/115419/#comment34588>
I wouldn't bother with this include. kcompletionmatches.h includes it, and this file does not make any direct use of this class, only of the KCompletionMatchesList typedef.
src/kcompletionmatches.cpp
<https://git.reviewboard.kde.org/r/115419/#comment34589>
Please put a comment by this include that it is for KCompletionMatchesWrapper, because it is not obvious why it would need the private header of KCompletion. The comment can literally just be
// for KCompletionMatchesWrapper
at the end of the line
- Alex Merry
On Feb. 4, 2014, 11:08 p.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, 11:08 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Repository: kcompletion
>
>
> Description
> -------
>
> To make code clearer, split KCompletionMatches.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt ae4a656
> src/kcompletion.h f197fc3
> src/kcompletion.cpp c684727
> src/kcompletion_p.h a8dedae
> src/kcompletionmatches.h PRE-CREATION
> src/kcompletionmatches.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/115419/diff/
>
>
> Testing
> -------
>
> It builds. Tests pass.
>
>
> Thanks,
>
> David Gil Oliva
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140204/fccd7e5a/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list