----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119970/#review65421 -----------------------------------------------------------
src/services/kplugininfo.h <https://git.reviewboard.kde.org/r/119970/#comment45701> 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; src/services/kplugininfo.cpp <https://git.reviewboard.kde.org/r/119970/#comment45702> I'd sort and reorder the includes (i.e. first Qt includes, then KDE includes). Also the other includes use <foo.h> style, so you should do the same for KPluginMetaData I guess src/services/kplugininfo.cpp <https://git.reviewboard.kde.org/r/119970/#comment45703> const& src/services/kplugininfo.cpp <https://git.reviewboard.kde.org/r/119970/#comment45704> 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"); ... } src/services/kplugininfo.cpp <https://git.reviewboard.kde.org/r/119970/#comment45705> ret.reserve(list.size()); src/services/kplugininfo.cpp <https://git.reviewboard.kde.org/r/119970/#comment45706> ret.reserve(list.size()); - Milian Wolff 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
