Review Request 118972: Don't have all the Thmeme color properties in the public api

[David Edmundson]

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

Ship it!


Concept seems fine in general.
Check my comment before pushing.


src/declarativeimports/core/quicktheme.h
<https://git.reviewboard.kde.org/r/118972/#comment42537>

    Doesn't redeclaring a signal in a subclass cause a massive warning at runtime?
    We used to do it somewhere else.
    
    I know it's needed for the Q_PROPERTIES because moc is rubbish but the only way I managed to solve this was to something like:
    
    Q_SIGNALS:
     void themeChangedProxy();
    
    and changing all the properties to use this.
    
    and in the ctor
    
    connect(this, signal("themeChanged"), this, signal("themeChangedProxy"));
    
    


- David Edmundson


On June 27, 2014, 10:30 a.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118972/
> -----------------------------------------------------------
> 
> (Updated June 27, 2014, 10:30 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> the color properties are remotely useful only when used from QML and risk to explode in number in the future, it's not a good thing for a public c++ api.
> 
> Also, they will become kinda deprecated as in largely replaced by the ColorRole api. Too late to remove those properties completely, but still last days to move them at least out of c++ api. (and in qml they could be removed in the future in a 2.1 version of the import while still being present in 2.0 if needed)
> 
> This moves the color properties is a theme subclass available only from QML (yes, it basically reintroduces the old themeproxy concept, but i don't think it was completely a good move to move everything in theme)
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/core/CMakeLists.txt e3d2f4e 
>   src/declarativeimports/core/corebindingsplugin.cpp ebd9fd7 
>   src/declarativeimports/core/quicktheme.h PRE-CREATION 
>   src/declarativeimports/core/quicktheme.cpp PRE-CREATION 
>   src/plasma/theme.h 0211f73 
>   src/plasma/theme.cpp c193807 
> 
> Diff: https://git.reviewboard.kde.org/r/118972/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140627/758e1896/attachment.html>