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

Milian Wolff mail at milianw.de
Thu Nov 6 10:54:18 UTC 2014


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



src/services/kplugininfo.cpp
<https://git.reviewboard.kde.org/r/120198/#comment48918>

    http://qt-project.org/doc/qt-5/qjsonobject.html#operator-5b-5d
    
    these should be 
    
    static const QString s_fooKey = QStringLiteral(...);
    
    better yet, wrap it all in an anonymous namespace and safe yourself the trouble of adding static everywhere



src/services/kplugininfo.cpp
<https://git.reviewboard.kde.org/r/120198/#comment48920>

    use QString for the key here as well



src/services/kplugininfo.cpp
<https://git.reviewboard.kde.org/r/120198/#comment48919>

    !json.contains



src/services/kplugininfo.cpp
<https://git.reviewboard.kde.org/r/120198/#comment48921>

    QString



src/services/kplugininfo.cpp
<https://git.reviewboard.kde.org/r/120198/#comment48923>

    this is more or less the same code as above, can you share it?



src/services/kplugininfo.cpp
<https://git.reviewboard.kde.org/r/120198/#comment48922>

    !contains



src/services/kplugininfo.cpp
<https://git.reviewboard.kde.org/r/120198/#comment48924>

    inline this as above, use QString and take the service smart ptr by const&


- Milian Wolff


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/20141106/f552b892/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list