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

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

directoryusagenotifier/cleanupdirectory.h (Diff revision 3)
public:
41
   explicit CleanUpDirectory(const KUrl &directory, const qint64 maximumSpaceUsed);
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 (Diff revision 3)
public:
56
  KFileItemList entriesToSort;
To be consistent, add m_

directoryusagenotifier/cleanupdirectory.cpp (Diff revision 3)
void CleanUpDirectory::deleteLeastUsed()
101
    if ( m_spaceCurrentlyUsed - spaceCleaned > m_maximumSpaceUsed ) {
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 (Diff revision 3)
29
#include <KJob>
#include <KDE/KJob> etc.

directoryusagenotifier/directoryusagenotifier.cpp (Diff revision 3)
DirectoryUsageNotifier::DirectoryUsageNotifier( QObject* parent )
74
    kDebug(7020) << "constructed with = " << dirs[0].mPath << "to check every "
My "kdebugdialog" says that 7020 is used for "kded4", you either need a different ID, or switch to dynamic IDs.

directoryusagenotifier/directoryusagenotifier.desktop (Diff revision 3)
4
Name[es]=Notificador de excesivo tamaƱo de directorio
It's nice that you want to help translating, but those translations are deleted on next scripty run :) Remove translations.

directoryusagenotifier/directoryusagenotifier.kcfg (Diff revision 3)
28
            <default>2</default>
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 (Diff revision 3)
37
      <string> MiB</string>
Is there a way to get the localized/customized suffix? I have my KDE set to "MB".

directoryusagenotifier/directoryusagenotifier_prefs_base.ui (Diff revision 3)
57
      <string>&lt;!DOCTYPE HTML PUBLIC &quot;-//W3C//DTD HTML 4.0//EN&quot; &quot;http://www.w3.org/TR/REC-html40/strict.dtd&quot;&gt;
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 September 2nd, 2011, 3:35 p.m., Jaime Torres Amate wrote:

Review request for kdelibs.
By Jaime Torres Amate.

Updated Sept. 2, 2011, 3:35 p.m.

Description

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?

Testing

It works as expected.
Bugs: 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)

View Diff