----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/4554/#review6587 -----------------------------------------------------------
Ship it! looks good; i have an idea for how to wrap all the loading code in the various plugin classes into this loader class. it's easier to do than explain and will only be a small tweak over this patch. i say commit once the apidox is complete :) trunk/KDE/kdelibs/plasma/applet.cpp <http://reviewboard.kde.org/r/4554/#comment6336> this check should probably still go first. trunk/KDE/kdelibs/plasma/externalpluginloader.h <http://reviewboard.kde.org/r/4554/#comment6337> the apidox should have parameter docs, e.g. @param name the name of the applet to load trunk/KDE/kdelibs/plasma/externalpluginloader.h <http://reviewboard.kde.org/r/4554/#comment6339> is there any need to use it with QVariant? points for thoroughness, but i'm not sure it's needed here :) trunk/KDE/kdelibs/plasma/externalpluginloader.cpp <http://reviewboard.kde.org/r/4554/#comment6338> i wonder if we should guard this so it can only be set once, e.g.: if (!s_externalPluginLoader) { s_externalPluginLoader = loader; } - Aaron On 2010-07-14 22:43:15, Ryan Rix wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/4554/ > ----------------------------------------------------------- > > (Updated 2010-07-14 22:43:15) > > > Review request for Plasma, Aaron Seigo, Robert Marmorstein, and Shaun Reich. > > > Summary > ------- > > Adds ability for applications which embed plasma to specify a class which > plasma will query for Applet, DataEngine, and Service creation, giving them > the chance to create these objects internally. > > The foremost use of this is to give Plasma the chance to ask applications > when restoring layouts from disk, for applets which were based on QWidgets, > such as the case in various Plasma dashboards which may embed QWidgets as > plasma::applets .... > > > Diffs > ----- > > trunk/KDE/kdelibs/plasma/CMakeLists.txt 1146180 > trunk/KDE/kdelibs/plasma/applet.cpp 1146180 > trunk/KDE/kdelibs/plasma/dataenginemanager.cpp 1146180 > trunk/KDE/kdelibs/plasma/externalpluginloader.h PRE-CREATION > trunk/KDE/kdelibs/plasma/externalpluginloader.cpp PRE-CREATION > trunk/KDE/kdelibs/plasma/service.cpp 1146180 > > Diff: http://reviewboard.kde.org/r/4554/diff > > > Testing > ------- > > Well, this thing is about 90% there right now, everything works, etc, afaict, > but I can't get the instance of the PluginManager outside of the Plasma > namespace, if someone could help that would be great. :) I can commit some > support into trunk/playground/base/shells/kpart > > > Thanks, > > Ryan > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
