Review Request 123626: port libplasma away from sycoca as much as possible

[Sebastian Kügler]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123626/#review79999
-----------------------------------------------------------

Ship it!


I've read and tested it, everything still works nicely. Good job!

I like how the patch moves much of the query language into simple C++, avoids a lot of query parsing and moves some of the error paths to compile time.


src/plasma/pluginloader.cpp 
<https://git.reviewboard.kde.org/r/123626/#comment54873>

    Why did you remove this, seems unrelated? (I'd keep the sorting in, just to assure backwards compat.)



src/plasma/pluginloader.cpp (line 730)
<https://git.reviewboard.kde.org/r/123626/#comment54874>

    Why? This remark makes it kinda magical and doesn't provide any other information than "rather don't touch this" which is counter-productive to actually fixing it.



src/plasma/pluginloader.cpp (line 796)
<https://git.reviewboard.kde.org/r/123626/#comment54875>

    Can this lead to containmentactions being listed twice? (I haven't seen it in the UI, but seems plausible as a corner case.)



src/plasma/private/applet_p.cpp (line 182)
<https://git.reviewboard.kde.org/r/123626/#comment54876>

    I think this may end up calling the non-const version, rather use foreach(const QString &..).



src/plasma/private/applet_p.cpp (line 461)
<https://git.reviewboard.kde.org/r/123626/#comment54877>

    redundant. Forgot to delete a row?



src/plasmapkg/plasmapkg.cpp (line 581)
<https://git.reviewboard.kde.org/r/123626/#comment54879>

    This can now be const


- Sebastian Kügler


On May 6, 2015, 5:21 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123626/
> -----------------------------------------------------------
> 
> (Updated May 6, 2015, 5:21 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> this ports most of libplasma away from sycoca, using instead a combination of KPluginLoader and KPackage::PackageLoader instead (so eventually using their own little caches instead of the global sycoca cache)
> a kservicetypetrader call is left in the loading of containmentactions since is the only way to make an older workspace still work, but is only a fallback, so containmentactions in plasma-workspace can be ported eventually as well
> 
> 
> Diffs
> -----
> 
>   KF5PlasmaConfig.cmake.in dee79ca 
>   src/declarativeimports/plasmacomponents/qml/private/TabBarLayout.qml be76a43 
>   src/plasma/applet.cpp 2dda381 
>   src/plasma/containment.cpp 7eabbb8 
>   src/plasma/containmentactions.cpp f24bdac 
>   src/plasma/dataengine.cpp 4b3d671 
>   src/plasma/package.cpp 4e0be88 
>   src/plasma/packagestructure.cpp b2686b6 
>   src/plasma/pluginloader.cpp 0ce349a 
>   src/plasma/private/applet_p.cpp 2cbfd06 
>   src/plasma/private/dataenginemanager.cpp b9c5d8e 
>   src/plasma/scripting/appletscript.h 863d707 
>   src/plasma/scripting/dataenginescript.h f7ad3c2 
>   src/plasma/scripting/scriptengine.cpp cc407e7 
>   src/plasma/service.h b5d9b5a 
>   src/plasma/service.cpp 3e9d852 
>   src/plasmapkg/plasmapkg.cpp d606365 
>   src/scriptengines/CMakeLists.txt f566406 
>   src/scriptengines/qml/CMakeLists.txt e7130db 
>   src/scriptengines/qml/plasmoid/appletinterface.cpp 07ecb3d 
>   src/scriptengines/qml/plasmoid/containmentinterface.cpp c6986f2 
>   src/scriptengines/qml/plasmoid/declarativeappletscript.cpp 24c39dd 
>   src/scriptengines/qml/plasmoid/wallpaperinterface.cpp a406d45 
> 
> Diff: https://git.reviewboard.kde.org/r/123626/diff/
> 
> 
> Testing
> -------
> 
> still needs a lot of testing, so far, with a master plasma-workspace notmal workspace functions seems fine
> 
> latest version still installs metadata files in kservices5. I would keep this for safety until Plasma 5.4 is released, then try to remove it again
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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