Review Request 112447: implement the PlasmaView library

[Marco Martin]

> 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 git at git.kde.org:clones/plasma-framework/tsiapaliwkas/pf5.git .
> 
> how to test it
> 
> * git clone git at 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130902/ba12c8e3/attachment-0001.html>