Review Request: add action collection to the corona

[Aaron Seigo]

-----------------------------------------------------------
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
> 
>