Review Request: TaskManager: Prompt user when automatic task to launcher mathing fails.

Aaron J. Seigo aseigo at kde.org
Fri Nov 4 11:29:13 UTC 2011


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


lots of coding style issues, though substance-wise it looks ok. a big concern i'm left with right now is that we end up with all of these patches in libtaskmanager and while it has more features, the overall quality of the code goes down due to the massive nature of the patches going in with very little testing combined with a decrease in readability due to less consistent coding style.

it also seems that in this patch we lose the ability to associate an executable with a launcher. that is a regression that needs addressing.

i also need to look at the config UI still, as well ...


libs/taskmanager/launcherproperties.cpp
<http://git.reviewboard.kde.org/r/103007/#comment6836>

    missing braces



libs/taskmanager/launcherproperties.cpp
<http://git.reviewboard.kde.org/r/103007/#comment6837>

    missing braces



libs/taskmanager/launcherproperties.cpp
<http://git.reviewboard.kde.org/r/103007/#comment6835>

    this should be the correct values, not "-1U". NET::WM2WindowClass for the third and 0 for the first parameter..


- Aaron J. Seigo


On Nov. 3, 2011, 9:21 p.m., Craig Drummond wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103007/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2011, 9:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> 1. If fail to automatically find launcher, then prompt user to select from installed applications.
> 2. Add a config page, so that manualy set launchers may be adjusted.
> 
> (Part of IconTasks' taskmanager changes)
> 
> 
> Diffs
> -----
> 
>   libs/taskmanager/CMakeLists.txt 6ae36dc 
>   libs/taskmanager/groupmanager.h acaa142 
>   libs/taskmanager/groupmanager.cpp 6e7ffa7 
>   libs/taskmanager/launcherconfig.h PRE-CREATION 
>   libs/taskmanager/launcherconfig.cpp PRE-CREATION 
>   libs/taskmanager/launcherconfig.ui PRE-CREATION 
>   libs/taskmanager/launcherproperties.h PRE-CREATION 
>   libs/taskmanager/launcherproperties.cpp PRE-CREATION 
>   libs/taskmanager/launcherproperties.ui PRE-CREATION 
>   libs/taskmanager/taskactions.cpp 011c565 
>   libs/taskmanager/taskactions_p.h 407b2c9 
>   libs/taskmanager/taskitem.h 3c84678 
>   libs/taskmanager/taskitem.cpp e2c07d8 
> 
> Diff: http://git.reviewboard.kde.org/r/103007/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Craig Drummond
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20111104/483fdebc/attachment.html>


More information about the Plasma-devel mailing list