Review Request 120198: KPluginInfo: use KPluginMetaData instead of a QVariantMap for storage

[Sebastian Kügler]

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


I've looked at the patch and tested it, it looks like a really useful improvement, and one more step of untangling the plugin / metadata situation from KService/KSycoca. I don't feel comfortable enough hitting ship it, but I don't see anything wrong with the patch, so +1.

I've been working on indexing the plugins for faster lookups, and going forward, this patch here is one of the steps that is really useful to my approach. You can find that in kservice[sebas/plugintrader] in KService, I've merged this patch and https://git.reviewboard.kde.org/r/120199/diff/# (the bit that makes KPluginTrader::query use KCoreAddons' KPluginLoader::findPlugins into that branch, so I can benefit from its improvements already, in fact they seem to be needed to not have to introduce further complexity there, and keep the delta as small as possible. This indexing doesn't really work, that is I had it working, and benchmarks already looked promising, but I've been reworking some bits to use KPluginMetaData, and in that process, it broke. I'm working on it. :)

- Sebastian Kügler


On Sept. 14, 2014, 2:05 p.m., Alexander Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120198/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2014, 2:05 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kservice
> 
> 
> Description
> -------
> 
> A series of 4 commits:
> 
> ----
> 
> KPluginInfo: use KPluginMetaData instead of a QVariantMap for storage
> 
> This means that KPluginInfo can now simply reuse the QJsonObject
> returned by QPluginLoader.metaData() (by storing it in a 
> KPluginMetaData object instead of having to convert the JSON to a
> QVariantMap first.
> 
> Additionally this allows very efficient conversion between KPluginInfo
> and KPluginMetaData.
> 
> ---
> Add compatibility key names to KPluginInfo::property()
> 
> ---
> 
> KPluginInfo: Fix loading JSON metadata that only has compatibility keys
> 
> This can be removed in KF6, but for now allows loading all both old
> style as well as new style metadata
> 
> ----
> 
> kplugininfotest: also test objects constructed from JSON
> 
> This tests both new style JSON as well as JSON using the old key names
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt 913e848ba5d1754ef7726f92604d1aaa398fa107 
>   autotests/kplugininfotest.cpp 34f87028ce08f2db1e5f57edbc6f99a237bf90ac 
>   src/services/kplugininfo.h dea07e6e63baf2483afc4a6d43d0892efc485903 
>   src/services/kplugininfo.cpp 50a6564edbbb1890c0b91badad69db967035231f 
> 
> Diff: https://git.reviewboard.kde.org/r/120198/diff/
> 
> 
> Testing
> -------
> 
> All unit tests still work
> 
> 
> Thanks,
> 
> Alexander Richardson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20141021/71855b53/attachment.html>