----------------------------------------------------------- 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 > >
