Review Request 110674: Moving KToolBarSpacerAction to kde4support

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110674/#review33247
-----------------------------------------------------------



staging/kde4support/src/kdeui/ktoolbarspaceraction.h
<http://git.reviewboard.kde.org/r/110674/#comment24594>

    That's so old, that line can definitely be removed.



staging/kde4support/src/kdeui/ktoolbarspaceraction.h
<http://git.reviewboard.kde.org/r/110674/#comment24593>

    @deprecated since 5.0, use XXX instead.
    
    What should the k3b, amarok, and kopete people use instead? They need to know...
    
    QToolBar::addSeparator? It doesn't have width/minWidth/maxWidth stuff.
    
    Hmm, OK, going through the uses of KToolBarSpacerAction in lxr.kde.org seems to indicate that none of the apps set these things anyway, so maybe addSeparator is enough indeed.
    
    Anyway - please always add @deprecated since 5.0 and the suggested replacement, for the benefit of people porting the code.



staging/kde4support/src/kdeui/ktoolbarspaceraction.h
<http://git.reviewboard.kde.org/r/110674/#comment24595>

    I'd suggest to remove virtual and append Q_DECL_OVERRIDE instead (new in Qt5), to ensure that this indeed reimplements something from a base class. Same for createWidget().


- David Faure


On May 27, 2013, 10:48 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110674/
> -----------------------------------------------------------
> 
> (Updated May 27, 2013, 10:48 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> Just moves the file, nobody seems to be using it in kdelibs anyway.
> 
> 
> Diffs
> -----
> 
>   kdeui/CMakeLists.txt dcbcb4f 
>   kdeui/actions/ktoolbarspaceraction.h 32e8fb2 
>   kdeui/actions/ktoolbarspaceraction.cpp db79af2 
>   staging/kde4support/src/CMakeLists.txt 1f6edde 
>   staging/kde4support/src/kdeui/ktoolbarspaceraction.h PRE-CREATION 
>   staging/kde4support/src/kdeui/ktoolbarspaceraction.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/110674/diff/
> 
> 
> Testing
> -------
> 
> Builds and installs fine.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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