[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request: windows port of taskmanager library
From:       "Aaron Seigo" <aseigo () kde ! org>
Date:       2009-08-02 23:04:33
Message-ID: 20090802230433.23325.73892 () localhost
[Download RAW message or body]


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


with a bit more work, the TaskManager class could also be made ifdef free. there are \
two blocks of code that need to be ifdef'd as far as i can see, and in them they \
check for just a few things: is the window set to skip the pager? is the window \
mapped? should the window be included in the task manager? a similar approach taken \
with Task could be taken with TaskManager. personally, i far prefer separating out \
the platform specific code like that; ifdef's tend to lead to untested branches and \
compile errors due to bit rot.

anyways, with a few more changes (noted below) this can go in. thanks for the patch \
:)


trunk/KDE/kdebase/workspace/libs/taskmanager/task.h
<http://reviewboard.kde.org/r/1101/#comment1236>

    this could easily be changed to:
    
    void addTransient(WId w, bool demandingAttention)
    
    the only thing that the NETWinInfo object is used for in addTransient is to check \
for attention demanding.  
    that would allow addTransient to be in both unix and win builds and one less \
ifdef.



trunk/KDE/kdebase/workspace/libs/taskmanager/task.cpp
<http://reviewboard.kde.org/r/1101/#comment1233>

    agreed



trunk/KDE/kdebase/workspace/libs/taskmanager/task.cpp
<http://reviewboard.kde.org/r/1101/#comment1232>

    instead of #include'ing the files here, this should be done in the CMakeLists.txt \
file



trunk/KDE/kdebase/workspace/libs/taskmanager/taskmanager.cpp
<http://reviewboard.kde.org/r/1101/#comment1235>

    is this needed?



trunk/KDE/kdebase/workspace/libs/taskmanager/taskmanager.cpp
<http://reviewboard.kde.org/r/1101/#comment1234>

    why is this #ifndef'd?


- Aaron


On 2009-07-22 23:57:29, Patrick Spendrin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1101/
> -----------------------------------------------------------
> 
> (Updated 2009-07-22 23:57:29)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> I want to add some missing functionality to the taskmanager library on the windows \
> platform. As the code has some more X11 dependencies, I decided to make up two \
> files for tasks.cpp which contain the functions that differ on Windows and X11. The \
> only thing that should affect X11-builds in the end should be the change from class \
> to struct for QUuid. There should be no other changes regarding the X11 platform.
> 
> 
> Diffs
> -----
> 
> trunk/KDE/kdebase/workspace/libs/CMakeLists.txt 1000690 
> trunk/KDE/kdebase/workspace/libs/taskmanager/task.h 1000690 
> trunk/KDE/kdebase/workspace/libs/taskmanager/task.cpp 1000690 
> trunk/KDE/kdebase/workspace/libs/taskmanager/task_win.cpp PRE-CREATION 
> trunk/KDE/kdebase/workspace/libs/taskmanager/task_x11.cpp PRE-CREATION 
> trunk/KDE/kdebase/workspace/libs/taskmanager/taskmanager.h 1000690 
> trunk/KDE/kdebase/workspace/libs/taskmanager/taskmanager.cpp 1000690 
> 
> Diff: http://reviewboard.kde.org/r/1101/diff
> 
> 
> Testing
> -------
> 
> I tested this on windows with both gcc and msvc compilers.
> 
> 
> Thanks,
> 
> Patrick
> 
> 

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


[prev in list] [next in list] [prev in thread] [next in thread]