----------------------------------------------------------- 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 > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
