> On Aug. 28, 2014, 2:07 nachm., Milian Wolff wrote: > > src/services/kplugininfo.h, line 429 > > <https://git.reviewboard.kde.org/r/119970/diff/1/?file=307970#file307970line429> > > > > here and below: > > > > maybe just rename this to > > > > toMetaData() > > fromMetaData() > > toMetaDataList() > > fromMetaDataList() > > > > Also, I'd probably remove the ...List from the name. C++ overload > > resolution can help with that. > > > > So you'd just have these: > > > > KPluginMetaData toMetaData() const; > > // to make the API symmetric also add this > > static KPluginMetaData toMetaData(const KPluginInfo& info) const; > > static KPluginInfo fromMetaData(const KPluginMetaData& metaData) > > const; > > static QVector<KPluginMetaData> toMetaData(const KPluginInfo::List& > > list) const; > > static KPluginInfo::List fromMetaData(const > > QVector<KPluginMetaData>& info) const;
Sounds good, will implement that > On Aug. 28, 2014, 2:07 nachm., 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"); > > ... > > } 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. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119970/#review65421 ----------------------------------------------------------- On Aug. 28, 2014, 1:13 nachm., Alexander Richardson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119970/ > ----------------------------------------------------------- > > (Updated Aug. 28, 2014, 1:13 nachm.) > > > 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
