Review Request 122232: KConfig: fix using KSharedConfig in global object destructor.

[Matthew Dawson]

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


The overall idea is sound.  Just a few issues:


autotests/kconfig_in_global_object.cpp
<https://git.reviewboard.kde.org/r/122232/#comment51750>

    This doesn't actually trigger anything when run without the fix below.  Since a name is provided in initConfig, it never tries to fetch the global name and thus doesn't test the issue.
    
    I think these two files should be the exact same, except for testing KSharedConfig vs KConfig.



autotests/ksharedconfig_in_global_object.cpp
<https://git.reviewboard.kde.org/r/122232/#comment51751>

    Just before this, could you stick a:
    qputenv("QT_FATAL_WARNINGS", "1");
    
    That way if a warning is printed the test will fail.  Otherwise this change might be removed with tests still passing.



src/core/kconfig.cpp
<https://git.reviewboard.kde.org/r/122232/#comment51752>

    Reading the Qt documentation, I think its possible that appArgs[0] isn't the application name on Windows, and thus appArgs may stay empty.
    
    Maybe add another conditional checking its empty in this if statement, and if it still is add some random value?


- Matthew Dawson


On Jan. 23, 2015, 5:39 p.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122232/
> -----------------------------------------------------------
> 
> (Updated Jan. 23, 2015, 5:39 p.m.)
> 
> 
> Review request for KDE Frameworks, Albert Astals Cid and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> -------
> 
> kconfig_in_global_object.cpp comes from kdelibs4support
> (after porting to Q_GLOBAL_STATIC)
> 
> ksharedconfig_in_global_object.cpp is new (but works with kdelibs4)
> and reproduces Albert's KgDifficulty testcase.
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt b91f754b705fc87bb8b729bea72fbb5f7d427ace 
>   autotests/kconfig_in_global_object.cpp PRE-CREATION 
>   autotests/ksharedconfig_in_global_object.cpp PRE-CREATION 
>   src/core/kconfig.cpp 782e9714521234a3e3d8f3a788967e5c9a40f38a 
> 
> Diff: https://git.reviewboard.kde.org/r/122232/diff/
> 
> 
> Testing
> -------
> 
> Both tests pass and the QCoreApplication::arguments warning (because called after qApp is destroyed) is gone.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150124/244658a0/attachment.html>