Review Request: new kded daemon to check .thumbnail directory space usage

[Christoph Feck]

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


This review may be incomplete, I mainly did it so that you get more feedback, ideally from others, too.


directoryusagenotifier/cleanupdirectory.h
<http://git.reviewboard.kde.org/r/102083/#comment5919>

    This should be the only constructor.
    
    If someone really needs to pass a QString, he can use CleanUpDirectory(KUrl(string)). This stresses the fact that he actually should have used KUrl in the first place.



directoryusagenotifier/cleanupdirectory.h
<http://git.reviewboard.kde.org/r/102083/#comment5920>

    To be consistent, add m_



directoryusagenotifier/cleanupdirectory.cpp
<http://git.reviewboard.kde.org/r/102083/#comment5922>

    This "if" has Space inside the braces. Since this is all new code, the coding style should be consistent. Since there are other style inconsistencies, I suggest to run it through astyle-kdelibs, which can be found in kdesdk/scripts.



directoryusagenotifier/directoryusagenotifier.h
<http://git.reviewboard.kde.org/r/102083/#comment5923>

    #include <KDE/KJob> etc.



directoryusagenotifier/directoryusagenotifier.cpp
<http://git.reviewboard.kde.org/r/102083/#comment5925>

    My "kdebugdialog" says that 7020 is used for "kded4", you either need a different ID, or switch to dynamic IDs.



directoryusagenotifier/directoryusagenotifier.desktop
<http://git.reviewboard.kde.org/r/102083/#comment5924>

    It's nice that you want to help translating, but those translations are deleted on next scripty run :) Remove translations.



directoryusagenotifier/directoryusagenotifier.kcfg
<http://git.reviewboard.kde.org/r/102083/#comment5928>

    If the default is to check every 2 minutes, then this is way too often. I cannot recommend a good default, but having my system cause disk contention by scanning for directory space usage every two minutes is inacceptable.



directoryusagenotifier/directoryusagenotifier_prefs_base.ui
<http://git.reviewboard.kde.org/r/102083/#comment5927>

    Is there a way to get the localized/customized suffix? I have my KDE set to "MB".



directoryusagenotifier/directoryusagenotifier_prefs_base.ui
<http://git.reviewboard.kde.org/r/102083/#comment5926>

    Please don't use HTML strings here. It specifies a fixed font and is horrible for translators. It is okey to add simple formatting using <qt> <br> <b> etc.


- Christoph


On Sept. 2, 2011, 3:35 p.m., Jaime Torres Amate wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102083/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2011, 3:35 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> This is not yet complete, it is missing the code to delete the files, trailing spaces and comments in spanish. (coming in next patch version)
> 
> There are also some things I'm not sure how should be done...
> The translations in .notifyrc and .desktop files, should be removed or just keep the lines with an empty traslation?
> Classes names, method names and variable names are OK?
> 
> 
> This addresses bug 79943.
>     http://bugs.kde.org/show_bug.cgi?id=79943
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt baf36cc 
>   directoryusagenotifier/CMakeLists.txt PRE-CREATION 
>   directoryusagenotifier/COPYING PRE-CREATION 
>   directoryusagenotifier/Messages.sh PRE-CREATION 
>   directoryusagenotifier/README PRE-CREATION 
>   directoryusagenotifier/cleanupdirectory.h PRE-CREATION 
>   directoryusagenotifier/cleanupdirectory.cpp PRE-CREATION 
>   directoryusagenotifier/directoryusagenotifier.h PRE-CREATION 
>   directoryusagenotifier/directoryusagenotifier.cpp PRE-CREATION 
>   directoryusagenotifier/directoryusagenotifier.desktop PRE-CREATION 
>   directoryusagenotifier/directoryusagenotifier.kcfg PRE-CREATION 
>   directoryusagenotifier/directoryusagenotifier.notifyrc PRE-CREATION 
>   directoryusagenotifier/directoryusagenotifier_prefs_base.ui PRE-CREATION 
>   directoryusagenotifier/module.h PRE-CREATION 
>   directoryusagenotifier/module.cpp PRE-CREATION 
>   directoryusagenotifier/settings.kcfgc PRE-CREATION 
>   directoryusagenotifier/tests/CMakeLists.txt PRE-CREATION 
>   directoryusagenotifier/tests/cleanupunittest.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102083/diff
> 
> 
> Testing
> -------
> 
> It works as expected.
> 
> 
> Thanks,
> 
> Jaime Torres
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110921/99b8d9c7/attachment.htm>