Review Request 119970: Add functions to convert between KPluginInfo and KPluginMetaData to KPluginInfo ( + unit test)

[Milian Wolff]

> On Aug. 28, 2014, 12:07 p.m., Milian Wolff wrote:
> > src/services/kplugininfo.cpp, line 523
> > <https://git.reviewboard.kde.org/r/119970/diff/1/?file=307971#file307971line523>
> >
> >     don't create the string literals multiple times, i.e. Name at least is twice here which increases the lib size. Personally I always put the stuff into
> >     
> >         namespace {
> >         const QString s_name = QStringLiteral("Name");
> >         ...
> >         }
> 
> Alexander Richardson wrote:
>     The problem here is that this creates a global constructor which increases library load time.
>     
>     I assumed the compiler would be clever enough to merge the string literals (even if they are unicode literals), but I didn't check it.

It won't merge it (at least GCC and Clang don't do it, afaik). If you worry about that, then at least just share the name, i.e. do somethig like

    static const QString name = QStringLiteral("Name");

and use that in both locations.


- Milian


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


On Aug. 28, 2014, 11:13 a.m., Alexander Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119970/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2014, 11:13 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kservice
> 
> 
> Description
> -------
> 
> - Add KPluginInfo::toKPluginMetaData and a unit test for it
> - Add KPluginInfo::fromKPluginMetaData and add a unit test for it
> - KPluginInfo: convert all meta data, not just the well known properties
> - Add functions to convert between lists of KPluginMetaData and KPluginInfo
> 
> This needs https://git.reviewboard.kde.org/r/119936
> 
> 
> Diffs
> -----
> 
>   autotests/kplugininfotest.cpp PRE-CREATION 
>   src/services/kplugininfo.h 9a9eceee5c90c6a5516c3b03473ff6437e9b2fe4 
>   src/services/kplugininfo.cpp 6fadf46c902455e7f5c9ece5b34fb1e40d0a97f7 
> 
> Diff: https://git.reviewboard.kde.org/r/119970/diff/
> 
> 
> Testing
> -------
> 
> Unit test passes, used successfully for loading KDevelop plugins with KPluginLoader::findPlugins()
> 
> 
> Thanks,
> 
> Alexander Richardson
> 
>

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