> On Sept. 2, 2013, 2:40 p.m., Aaron J. Seigo wrote: > > the Plasma/Shell package definition should live in libplasma along with the > > others. this will remove the need for the PluginLoader subclass and ensure > > that Corona can be used by any application that simply links to libplasma, > > which is obviously a requirement. > > > > the other classes should perhaps find a more appropriate home as well > > rather than creating a whole new library. really, this is not the "Plasma > > View Library" but the "if you are writing a QML shell with libplasma you > > will find these couple of classes useful..." library. most optimal would be > > if this could be tucked away with the rest of the QML support for plasma, > > perhaps in the script engine library itself. > > > > the config classes also scream out to be made internal and not public API. > > as the main class used is a QQuickView, putting this in the QML support > > would be sensible and then all of this could be nicely hidden away. > > > > the classes also seem to be in need of some API review before we commit to > > binary compatibility for them ... something for one of the upcoming monday > > meetings?
in master most of issues should be addressed now > On Sept. 2, 2013, 2:40 p.m., Aaron J. Seigo wrote: > > src/plasmaview/configview.h, line 169 > > <http://git.reviewboard.kde.org/r/112447/diff/1/?file=186293#file186293line169> > > > > this TODO seems to be done confirmed is a done todo > On Sept. 2, 2013, 2:40 p.m., Aaron J. Seigo wrote: > > src/plasmaview/configview.h, line 170 > > <http://git.reviewboard.kde.org/r/112447/diff/1/?file=186293#file186293line170> > > > > is this TODO still valid given that this code is moving into a library? confirmed is a done todo > On Sept. 2, 2013, 2:40 p.m., Aaron J. Seigo wrote: > > src/plasmaview/containmentconfigview_p.h, line 34 > > <http://git.reviewboard.kde.org/r/112447/diff/1/?file=186295#file186295line34> > > > > another stray TODO? confirmed is a done todo > On Sept. 2, 2013, 2:40 p.m., Aaron J. Seigo wrote: > > src/plasmaview/containmentconfigview_p.cpp, lines 51-54 > > <http://git.reviewboard.kde.org/r/112447/diff/1/?file=186296#file186296line51> > > > > why is this using Plasma/Generic instead of a Plasma/QmlWallpaper (or > > whatever) package? > > > > sprinkling our code with setDefaultPackageRoot and filePath calls with > > literal values defeats the purpose of Plasma::Package: to encapsulate those > > details. > > > > if we were to move where the wallpaper packages were kept, then what? > > we search every single bit of code that might use wallpapers? > > > > no, that's why we have Plasma::Package subclasses! there should be a > > package type for these wallpapers. I now made a QmlWallpaper packagestructure locally in the shell, now it uses that (it pretty much duplicates Generic) - Marco ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112447/#review39164 ----------------------------------------------------------- On Sept. 2, 2013, 12:53 p.m., Giorgos Tsiapaliokas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112447/ > ----------------------------------------------------------- > > (Updated Sept. 2, 2013, 12:53 p.m.) > > > Review request for Plasma. > > > Description > ------- > > This patch creates a new library out of the shell dir. > We need this library in order to implement plasmoidviewer 2.0 > > > Diffs > ----- > > src/CMakeLists.txt 281d146 > src/plasmaview/CMakeLists.txt PRE-CREATION > src/plasmaview/PlasmaViewConfig.cmake.in PRE-CREATION > src/plasmaview/configview.h PRE-CREATION > src/plasmaview/configview.cpp PRE-CREATION > src/plasmaview/containmentconfigview_p.h PRE-CREATION > src/plasmaview/containmentconfigview_p.cpp PRE-CREATION > src/plasmaview/currentcontainmentactionsmodel_p.h PRE-CREATION > src/plasmaview/currentcontainmentactionsmodel_p.cpp PRE-CREATION > src/plasmaview/includes/PlasmaView/ConfigView PRE-CREATION > src/plasmaview/includes/PlasmaView/ContainmentConfigView PRE-CREATION > src/plasmaview/includes/PlasmaView/ShellPluginLoader PRE-CREATION > src/plasmaview/includes/PlasmaView/View PRE-CREATION > src/plasmaview/shellpackage_p.h PRE-CREATION > src/plasmaview/shellpackage_p.cpp PRE-CREATION > src/plasmaview/shellpluginloader.h PRE-CREATION > src/plasmaview/shellpluginloader.cpp PRE-CREATION > src/plasmaview/view.h PRE-CREATION > src/plasmaview/view.cpp PRE-CREATION > src/shell/CMakeLists.txt 3da019f > src/shell/configview.h 2e8f68f > src/shell/configview.cpp fea5a73 > src/shell/containmentconfigview.h 619fa14 > src/shell/containmentconfigview.cpp 235a33f > src/shell/currentcontainmentactionsmodel.h db94da1 > src/shell/currentcontainmentactionsmodel.cpp c955bef > src/shell/shellcorona.cpp ffdbfe8 > src/shell/shellpackage.h 99dc460 > src/shell/shellpackage.cpp 74aea5c > src/shell/shellpluginloader.h 1d3dade > src/shell/shellpluginloader.cpp 8b2e1dd > src/shell/view.h 7e6b2d9 > src/shell/view.cpp a0c6168 > > Diff: http://git.reviewboard.kde.org/r/112447/diff/ > > > Testing > ------- > > The code is locate at > g...@git.kde.org:clones/plasma-framework/tsiapaliwkas/pf5.git . > > how to test it > > * git clone g...@git.kde.org:clones/plasma-framework/tsiapaliwkas/pf5.git > * cd pf5 > * git checkout split7 > * build it and install it > * use plasma-shell > > I haven't noticed any issues. > > > Thanks, > > Giorgos Tsiapaliokas > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
