Review Request: Command line option to sweeper (Bug 230568)

[Raphael Kubo da Costa]

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



/trunk/KDE/kdeutils/sweeper/main.cpp
<http://reviewboard.kde.org/r/3820/#comment5380>

    Trailing whitespace.



/trunk/KDE/kdeutils/sweeper/main.cpp
<http://reviewboard.kde.org/r/3820/#comment5382>

    Using a bool here makes it a bit harder to understand the code: at first glance, how would one know why true or false is being passed?
    
    I'm in favour of leaving the constructor without any parameter and using something like:
    
    if (args->isSet(QLatin1String("automatic"))) {
      app->setAutomaticMode(true);
      app->cleanup();
      app->close();
    } else
      app->show();
    
    In order to implement that suggestion I gave about not showing the window, you could move the cleanup code somewhere else, so that the KXmlGuiWindow is separated from the cleanup code itself. But that'd require quite big a patch. IMO, we can work a bit more on this one, and then implement this later.



/trunk/KDE/kdeutils/sweeper/main.cpp
<http://reviewboard.kde.org/r/3820/#comment5381>

    Trailing whitespace.


- Raphael


On 2010-05-04 17:17:56, xeike wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3820/
> -----------------------------------------------------------
> 
> (Updated 2010-05-04 17:17:56)
> 
> 
> Review request for kdeutils.
> 
> 
> Summary
> -------
> 
> Hi!
> 
> The patch adds a command line opton to sweeper. Using this command line option, sweeper starts, sweeps and finishes without user interaction.
> Instad of the suggested "--clear-all", I just use "automatic", because the user can edit what to clear during an interactive session.
> 
> 
> This addresses bug 230568.
>     https://bugs.kde.org/show_bug.cgi?id=230568
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeutils/sweeper/main.cpp 1122773 
>   /trunk/KDE/kdeutils/sweeper/sweeper.h 1122773 
>   /trunk/KDE/kdeutils/sweeper/sweeper.cpp 1122773 
> 
> Diff: http://reviewboard.kde.org/r/3820/diff
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> xeike
> 
>