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