Review Request 111908: Add KConfigLoader from Plasma Framework to KConfigGui

David Faure faure at kde.org
Fri Aug 9 19:27:23 UTC 2013


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



tier1/kconfig/src/gui/kconfigloader.h
<http://git.reviewboard.kde.org/r/111908/#comment27699>

    I wonder if this should really derive from KConfigSkeleton, rather than encapsulate it.
    
    For instance it means you can't get a "core only" version of it (see KCoreConfigSkeleton).
    
    It doesn't reimplement any virtual methods, so this inheritance seems unnecessary. On the other hand I'm not sure how it would work by composition anyway; maybe the caller would have to create it themselves (which would be a bit cumbersome?).
    
    The use of QColor in the code makes core/gui separation difficult anyway, it would need to be provided by a runtime hook.
    
    Just an idea.
    Don't know how much core/gui separation is useful for this class.


- David Faure


On Aug. 9, 2013, 12:38 p.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111908/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2013, 12:38 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Aaron J. Seigo.
> 
> 
> Description
> -------
> 
> Add KConfigLoader from Plasma Framework to KConfigGui
> 
> The ConfigLoader is way to awesome to not be directly in KConfig.
> 
> 
> Diffs
> -----
> 
>   tier1/kconfig/autotests/CMakeLists.txt c913da3 
>   tier1/kconfig/autotests/kconfigloadertest.h PRE-CREATION 
>   tier1/kconfig/autotests/kconfigloadertest.cpp PRE-CREATION 
>   tier1/kconfig/autotests/kconfigloadertest.xml PRE-CREATION 
>   tier1/kconfig/src/gui/CMakeLists.txt 0913349 
>   tier1/kconfig/src/gui/kconfigloader.h PRE-CREATION 
>   tier1/kconfig/src/gui/kconfigloader.cpp PRE-CREATION 
>   tier1/kconfig/src/gui/kconfigloader_p.h PRE-CREATION 
>   tier1/kconfig/src/gui/kconfigloaderhandler_p.h PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/111908/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

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


More information about the Kde-frameworks-devel mailing list