Review Request: When a source file for kconf_update does not exist, do not create it

[Aurélien Gâteau]

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


Looks good. I find the way gotFile() is used to flush the changes a bit weird, but it was there before your changes.
Would be great to add a unit-test if possible though.


/trunk/KDE/kdelibs/kconf_update/kconf_update.cpp
<http://reviewboard.kde.org/r/4243/#comment5622>

    Nitpick: Keep this include with the other QtCore includes



/trunk/KDE/kdelibs/kconf_update/kconf_update.cpp
<http://reviewboard.kde.org/r/4243/#comment5619>

    Nitpick: Please fix indentation for these lines



/trunk/KDE/kdelibs/kconf_update/kconf_update.cpp
<http://reviewboard.kde.org/r/4243/#comment5620>

    Would be good to have different messages at line 520 and 525. It can make debugging a bit easier.


- Aurélien


On 2010-06-06 13:57:27, Kevin Krammer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4243/
> -----------------------------------------------------------
> 
> (Updated 2010-06-06 13:57:27)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Currently, when kconf_update is being run by kdeinit4 on a new user account, it creates all config files that it has updates for, regardless whether these files exist.
> Which mean .kde/share/config gets populated with a lot of files with only kconf_update's entry in them.
> 
> Unfortunately that means that programs checking for a specific file's existance, e.g. PIM apps which are checking whether to run a migrator tool, will be fooled into assuming that there are actually valid settings.
> 
> The patch uses the already set m_skipFile flag to decide when not to sync the KConfig object, thus not creating the file if it didn't exist before
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kconf_update/kconf_update.cpp 1135120 
> 
> Diff: http://reviewboard.kde.org/r/4243/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin
> 
>