----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/268/#review441 -----------------------------------------------------------
needs some clean ups here and there, but the general direction seems correct; i do agree with Marco that an accessor to the full Corona action collection will be desirable (e.g. for context menu plugins) trunk/KDE/kdelibs/plasma/containment.cpp <http://reviewboard.kde.org/r/268/#comment275> i suppose that this will eventually move to the contxt menu plugins? trunk/KDE/kdelibs/plasma/containment.cpp <http://reviewboard.kde.org/r/268/#comment274> should have an if (lockDesktopAction) here trunk/KDE/kdelibs/plasma/containment.cpp <http://reviewboard.kde.org/r/268/#comment269> well, without the lockDesktopAction here, nobody is using it and the whole method could just as well be removed. trunk/KDE/kdelibs/plasma/containment.cpp <http://reviewboard.kde.org/r/268/#comment270> it can be. the other approach is to do a Q_ASSERT(q->corona()) at the top of the method. i tend to be somewhat cautious of such things, however. trunk/KDE/kdelibs/plasma/corona.h <http://reviewboard.kde.org/r/268/#comment271> setImmutability is enough imho. no need for toggleImmutability to be publicly exposed. trunk/KDE/kdelibs/plasma/corona.cpp <http://reviewboard.kde.org/r/268/#comment272> none of the private objects are, they use the object they are private to for qobject facilities. this avoids unnecessary overhead and various complexity due to having signals/slots spread over multiple classes that are really just one. trunk/KDE/kdelibs/plasma/corona.cpp <http://reviewboard.kde.org/r/268/#comment276> should be a private slot for use by the action in Corona only, just as it was in Containment - Aaron On 2009-03-09 10:24:36, Chani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/268/ > ----------------------------------------------------------- > > (Updated 2009-03-09 10:24:36) > > > Review request for Plasma and Marco Martin. > > > Summary > ------- > > some of the actions in Containment don't belong there, and there'll probably > be more of those after the summer, so let's give them a home in Corona. > I've only moved over the lock action so far; I'll move the "new activity" one > next. > Containment can grab the actions from the corona so the UI isn't affected by > this patch and nothing breaks. there's just less duplicate actions. I'll > leave the UI changes to notmart. :) > > > Diffs > ----- > > trunk/KDE/kdelibs/plasma/containment.cpp 936946 > trunk/KDE/kdelibs/plasma/corona.h 936946 > trunk/KDE/kdelibs/plasma/corona.cpp 936946 > > Diff: http://reviewboard.kde.org/r/268/diff > > > Testing > ------- > > works with desktop and screensaver. > > > Thanks, > > Chani > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
