> On May 7, 2015, 7:24 a.m., Alex Richardson wrote: > > Seems like this is duplicated in a few places already so I agree we should > > add it. But won't most users of the API want only a single plugin returned? > > Maybe also add a function `KPluginMetaData > > KPluginLoader::findPluginById(const QString& directory, const QString& > > pluginId)`? > > Do we also need the function that returns a vector for a given ID? > > Sebastian Kügler wrote: > At least our changes in libplasma need a QVector<KPluginMetaData>. > Otherwise, a list seems easy enough to check if something's found. Returning > a single metadata won't be very useful for us at this point (but I see it > making sense).
Ow, also, and perhaps more importantly, multiple ids are technically perfectly valid (only the plugin name and directory are important here). I think that fact should be reflected in the API. Perhaps a word on ordering would be in place in the API docs, plugin locations are cascaded properly in code using it. The most local plugin is at the end of the list, system-widely installed ones at the beginning, so code that uses plugins.first() would not allow the user to override plugins installed for example into /usr/lib with a plugin with the same id and relative path installed into ~/.local). So an extra method returning the .last() plugin found might take away this caveat from the API. We'll still need the method returning a vector for libplasma, though (and I think it's a semantically useful addition). About adding another method to return the most-local plugin, I'm on the edge. If others think it's useful and we think the additional API (with its long-term maintainance implications) is worth it, I'm happy to add it as well. (Perhaps in a separate review.) Opinions welcome. - Sebastian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123669/#review80013 ----------------------------------------------------------- On May 6, 2015, 11:21 p.m., Sebastian Kügler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123669/ > ----------------------------------------------------------- > > (Updated May 6, 2015, 11:21 p.m.) > > > Review request for KDE Frameworks, Alex Richardson and David Faure. > > > Repository: kcoreaddons > > > Description > ------- > > Add findPluginsById convenience API > > It's a quite common case to load a plugin from an ID. This makes it > easy. > > CHANGELOG:New KPluginLoader::findPluginById() convenience API > REVIEW:123669 > > > Diffs > ----- > > autotests/kpluginloadertest.cpp 3ded0ebca2e0fd20e09bf6e4eca152d13ac11f46 > src/lib/plugin/kpluginloader.h 124bfab7e207b17d3c8ab4d5a88321af476aad42 > src/lib/plugin/kpluginloader.cpp 4310d6cd7792329511f12b28d7c68b0fdd013538 > > Diff: https://git.reviewboard.kde.org/r/123669/diff/ > > > Testing > ------- > > Added autotests, everything passes. > > > Thanks, > > Sebastian Kügler > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
