Review Request 124331: New proxy: KExtraColumnsProxyModel, allows to add columns to an existing model.

Milian Wolff mail at milianw.de
Tue Jul 14 16:58:12 UTC 2015


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124331/#review82500
-----------------------------------------------------------

Ship it!



src/kextracolumnsproxymodel.cpp (line 36)
<https://git.reviewboard.kde.org/r/124331/#comment56910>

    the slots here is not required, no? only in the public part? if it's required, then you also need a 'public:' section below for the data members



src/kextracolumnsproxymodel.cpp (line 44)
<https://git.reviewboard.kde.org/r/124331/#comment56908>

    these two really should be QVector. As far as I can see, you do not interact with the Qt API and thus need QList for them, or am I missing something? QModelIndex is too large, and thus you'll allocate every item in the list on the heap. int is, on 64bit, too small and thus you waste space (factor of 2).



src/kextracolumnsproxymodel.cpp (line 85)
<https://git.reviewboard.kde.org/r/124331/#comment56909>

    here and elsewhere: why not use the newstyle connect syntax?


- Milian Wolff


On July 14, 2015, 10:42 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124331/
> -----------------------------------------------------------
> 
> (Updated July 14, 2015, 10:42 a.m.)
> 
> 
> Review request for KDE Frameworks, Jesper Pedersen and Stephen Kelly.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> -------
> 
> REVIEW: 124331
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt f98e87d49b965998f31c5ede1fefb03b159a150f 
>   autotests/kextracolumnsproxymodeltest.cpp PRE-CREATION 
>   autotests/test_model_helpers.h a44db8a9cb985f69b52be96f0ffe389ebd903d6f 
>   src/CMakeLists.txt 82d776f1fcc2f7b15e74f0ed47702ed570ce9892 
>   src/kextracolumnsproxymodel.h PRE-CREATION 
>   src/kextracolumnsproxymodel.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124331/diff/
> 
> 
> Testing
> -------
> 
> Wrote autotests (as extensive as possible); used this in one real project, AFAIK Jesper (blackie) did as well.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150714/7e9c41ec/attachment.html>


More information about the Kde-frameworks-devel mailing list