----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/4585/#review6545 -----------------------------------------------------------
/trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupableitem.h <http://reviewboard.kde.org/r/4585/#comment6289> almost sounds like we should replace isGroupItem() with a itemType() method that returns a Type enum. would allow us to more easily add new features in the future, and keeps the virtual table smaller. /trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupableitem.h <http://reviewboard.kde.org/r/4585/#comment6288> please put the (even empty :) implementation into the .cpp file. /trunk/KDE/kdebase/workspace/libs/taskmanager/groupmanager.h <http://reviewboard.kde.org/r/4585/#comment6290> should be "const KUrl &url" /trunk/KDE/kdebase/workspace/libs/taskmanager/groupmanager.cpp <http://reviewboard.kde.org/r/4585/#comment6291> no spaces before/after parens. may be worth caching item->task()->classClass().toLower() /trunk/KDE/kdebase/workspace/libs/taskmanager/groupmanager.cpp <http://reviewboard.kde.org/r/4585/#comment6292> same as comments on line 290 above. /trunk/KDE/kdebase/workspace/libs/taskmanager/groupmanager.cpp <http://reviewboard.kde.org/r/4585/#comment6293> looks indented incorrectly? /trunk/KDE/kdebase/workspace/libs/taskmanager/launcheritem.h <http://reviewboard.kde.org/r/4585/#comment6294> should be BSD'd like the rest of the lib. /trunk/KDE/kdebase/workspace/libs/taskmanager/launcheritem.h <http://reviewboard.kde.org/r/4585/#comment6295> please don't line up method names :) /trunk/KDE/kdebase/workspace/libs/taskmanager/launcheritem.cpp <http://reviewboard.kde.org/r/4585/#comment6297> would be very cool if this tracking could be encapsulated in libtaskamanager itself, but that can always be a second step... /trunk/KDE/kdebase/workspace/libs/taskmanager/launcheritem.cpp <http://reviewboard.kde.org/r/4585/#comment6296> should just be: return d->windowInstances == 0; /trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/programgroupingstrategy.cpp <http://reviewboard.kde.org/r/4585/#comment6300> there's no point in doing a dynamic_cast unless you check the value. in any case, taskItem already is a TaskItem in this context. looks like an unnecessary change :) /trunk/KDE/kdebase/workspace/libs/taskmanager/taskgroup.cpp <http://reviewboard.kde.org/r/4585/#comment6298> space between foreach and the opening ( /trunk/KDE/kdebase/workspace/libs/taskmanager/taskgroup.cpp <http://reviewboard.kde.org/r/4585/#comment6299> why are launchers with no window instances removed from the member list? - Aaron On 2010-07-10 18:21:39, Anton Kreuzkamp wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/4585/ > ----------------------------------------------------------- > > (Updated 2010-07-10 18:21:39) > > > Review request for Plasma. > > > Summary > ------- > > Adds support for Windows 7 like launchers in libtaskmanager. > (I'm on holliday from 12th July until 1st August so I will not be able to > reply during this time.) > > > Diffs > ----- > > /trunk/KDE/kdebase/workspace/libs/taskmanager/taskitem.h 1148442 > /trunk/KDE/kdebase/workspace/libs/taskmanager/taskgroup.cpp 1148442 > /trunk/KDE/kdebase/workspace/libs/taskmanager/taskgroup.h 1148442 > > /trunk/KDE/kdebase/workspace/libs/taskmanager/strategies/programgroupingstrategy.cpp > 1148442 > /trunk/KDE/kdebase/workspace/libs/taskmanager/launcheritem.cpp PRE-CREATION > /trunk/KDE/kdebase/workspace/libs/taskmanager/launcheritem.h PRE-CREATION > /trunk/KDE/kdebase/workspace/libs/taskmanager/groupmanager.cpp 1148442 > /trunk/KDE/kdebase/workspace/libs/taskmanager/groupmanager.h 1148442 > /trunk/KDE/kdebase/workspace/libs/taskmanager/abstractgroupableitem.h > 1148442 > /trunk/KDE/kdebase/workspace/libs/taskmanager/abstractsortingstrategy.cpp > 1148442 > /trunk/KDE/kdebase/workspace/libs/taskmanager/CMakeLists.txt 1148442 > > Diff: http://reviewboard.kde.org/r/4585/diff > > > Testing > ------- > > Tested with a small test-applett and everything works. > > > Thanks, > > Anton > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
