Review Request 115370: Fix apidox, fix code style and delete useless includes in KComboBox (KCompletion)
Alex Merry
kde at randomguy3.me.uk
Fri Jan 31 20:07:25 UTC 2014
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115370/#review48708
-----------------------------------------------------------
Comments on the dox changes below. Please make the changes to the .cpp file a separate review request, though.
src/kcombobox.h
<https://git.reviewboard.kde.org/r/115370/#comment34383>
These are no longer really true. Replace with something lik "A combo box with completion support."
src/kcombobox.h
<https://git.reviewboard.kde.org/r/115370/#comment34384>
Remove
src/kcombobox.h
<https://git.reviewboard.kde.org/r/115370/#comment34382>
funtionalities -> features
src/kcombobox.h
<https://git.reviewboard.kde.org/r/115370/#comment34385>
This sounds weird; remove indefinite articles ("a"), or make it "Additionally, the returnPressed() and returnPressed(const QString&)..."
src/kcombobox.h
<https://git.reviewboard.kde.org/r/115370/#comment34386>
no comma, and I would remove the word "simply"
src/kcombobox.h
<https://git.reviewboard.kde.org/r/115370/#comment34387>
"or rather select-only" should be in parentheses, like "(or rather select-only)".
I would also remove the "with a parent object" bit; this is a common pattern for QWidgets and does not need to be stated here.
src/kcombobox.h
<https://git.reviewboard.kde.org/r/115370/#comment34388>
I would remove everything after "read-only combo box"
src/kcombobox.h
<https://git.reviewboard.kde.org/r/115370/#comment34392>
Use the terminology "editable" more consistently? Instead of read-write.
src/kcombobox.h
<https://git.reviewboard.kde.org/r/115370/#comment34389>
Why? This is out of keeping with every other method's documentation...
src/kcombobox.h
<https://git.reviewboard.kde.org/r/115370/#comment34390>
While not strictly necessary, a line break after the first sentence makes it clearer to read (same goes for dox of other methods).
I would also generally prefer a dox style where the description was like
"Appends a URL with an icon to the combobox."
and there were @param entries. But I'm not going to reject the patch for not doing this.
src/kcombobox.h
<https://git.reviewboard.kde.org/r/115370/#comment34391>
"editable" instead of "read-writable"?
- Alex Merry
On Jan. 28, 2014, 10:44 p.m., David Gil Oliva wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115370/
> -----------------------------------------------------------
>
> (Updated Jan. 28, 2014, 10:44 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Repository: kcompletion
>
>
> Description
> -------
>
> Fix apidox, fix code style and delete useless includes.
>
>
> Diffs
> -----
>
> src/kcombobox.h f34d259
> src/kcombobox.cpp 2cfe6e7
>
> Diff: https://git.reviewboard.kde.org/r/115370/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/20140131/287bdee9/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list