Review Request 112363: Make it possible for QToolBar to use "Other toolbar" settings

[David Faure]

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


Looks good, just minor things found


KDE5PORTING.html
<http://git.reviewboard.kde.org/r/112363/#comment28746>

    punctuation makes this hard to read.
    
    My suggestion:
    
    KToolBar has moved to XMLGUI. If you don't want to depend on XMLGUI, use QToolBar instead, with the following setup:



KDE5PORTING.html
<http://git.reviewboard.kde.org/r/112363/#comment28745>

    s/on KStyle/when using a widget style that derives from KStyle/



staging/frameworkintegration/autotests/kstyle_unittest.cpp
<http://git.reviewboard.kde.org/r/112363/#comment28747>

    *Nothing* uses KDEHOME anymore. I thought I killed all instances; where does this come from?



staging/frameworkintegration/autotests/kstyle_unittest.cpp
<http://git.reviewboard.kde.org/r/112363/#comment28748>

    Why not just use QStandardPaths::setTestModeEnabled(true)?



staging/frameworkintegration/autotests/kstyle_unittest.cpp
<http://git.reviewboard.kde.org/r/112363/#comment28750>

    covered by qstandardpaths too



staging/frameworkintegration/autotests/kstyle_unittest.cpp
<http://git.reviewboard.kde.org/r/112363/#comment28749>

    doesn't exist anywhere anymore (replaced with $QT_MESSAGE_PATTERN)



staging/frameworkintegration/src/kstyle/kstyle.cpp
<http://git.reviewboard.kde.org/r/112363/#comment28751>

    I *think* QLatin1String is better for this case (a simple comparison), since there's an overload for QString::operator==(QLatin1String).



staging/xmlgui/src/ktoolbar.h
<http://git.reviewboard.kde.org/r/112363/#comment28753>

    same here


- David Faure


On Aug. 29, 2013, 4:13 p.m., Àlex Fiestas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112363/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2013, 4:13 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> Qt only knows about one kind of Toolbar while in KDE we know about
> two, one we call "Main Toolbar" and another one called "Other Toolbars".
>     
> This commit adds a small hack that makes it possible for an application
> not using KToolbar (for example beacuse it can't depend on XMLGui) use
> "Other Toolbar" by setting the dynamic property "otherToolbar" to any
> value, for example true.
> 
> 
> Diffs
> -----
> 
>   KDE5PORTING.html dcffa27 
>   staging/frameworkintegration/autotests/CMakeLists.txt d91eb7a 
>   staging/frameworkintegration/autotests/kdeplatformtheme_kdeglobals 351074b 
>   staging/frameworkintegration/autotests/kstyle_unittest.cpp PRE-CREATION 
>   staging/frameworkintegration/src/kstyle/kstyle.cpp 7257b9d 
>   staging/xmlgui/src/ktoolbar.h d786a17 
> 
> Diff: http://git.reviewboard.kde.org/r/112363/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Àlex Fiestas
> 
>

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