Review Request 112680: KPluginTrader: QPluginLoader/metadata-based querying of Plugins

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112680/#review39865
-----------------------------------------------------------



staging/kservice/src/plugin/kplugintrader.h
<http://git.reviewboard.kde.org/r/112680/#comment29399>

    I guess at this point we can remove references to CORBA :-)
    
    (This was initially written because it was replacing the pre-kde-2.0 corba trader)



staging/kservice/src/plugin/kplugintrader.h
<http://git.reviewboard.kde.org/r/112680/#comment29400>

    same here (and the one before) : I'd swap subDirectory and serviceType.
    
    We really want to encourage people to pass a subdirectory. The performance would be horrendous otherwise.



staging/kservice/src/plugin/kplugintrader.cpp
<http://git.reviewboard.kde.org/r/112680/#comment29396>

    the var name needs adjusting (copy/pasted from kservicetypetrader)



staging/kservice/src/plugin/kplugintrader.cpp
<http://git.reviewboard.kde.org/r/112680/#comment29398>

    s/( /(/ and s/ )/)/ everywhere.



staging/kservice/src/plugin/kplugintrader.cpp
<http://git.reviewboard.kde.org/r/112680/#comment29397>

    Do we want to support the "simple case" of servicetype being empty? I.e. "load all plugins from subDirectory", without an additional servicetype constraint.
    
    It seems to me that this would simplify a number of use cases like
    kf5/thumbcreator/
    kf5/kcmodules/
    kf5/kurifilters/
    i.e. all the cases were the type of service is already clear from the subdir name.
    
    This is in contrast with kf5/calligra/ unless they also use one subdir per servicetype, it's also in contrast with KParts where a part implements multiple servicetypes (readonly part, readwrite part, browserview) etc. The servicetype argument (and the servicetype in the metadata) is still useful for these cases, but it's just redundant for the "simple cases" (which might actually be 90% of the cases...)
    
    Actually this might even be a reason to swap the two arguments:
    subDirectory, servicetype = QString(), constraint = QString()
    



staging/kservice/src/plugin/kplugintrader.cpp
<http://git.reviewboard.kde.org/r/112680/#comment29401>

    Remind me, why isn't this just a constructor argument? That would solve the issues with modifying an explicitly-shared-object.
    


- David Faure


On Sept. 12, 2013, 3:12 a.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112680/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2013, 3:12 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Description
> -------
> 
> A trader interface (similar to the CORBA Trader), which provides a way query specific subdirectories in the Qt plugin paths for plugins. KPluginTrader provides an easy way to load a plugin instance from a KPluginFactory, just querying for existing plugins.
> 
> KPluginTrader provides a way for an application to query directories in the Qt plugin paths, accessed through QCoreApplication::libraryPaths(). Plugins may match a specific set of requirements. This allows to find specific plugins at run-time without having to hard-code their names and/or paths. KPluginTrader does not search recursively, you are rather encouraged to install plugins into specific subdirectories to further speed searching.
> 
> KPluginTrader exclusively searches within the plugin binaries' metadata (via QPluginLoader::metaData()). It does not search these directories recursively and it does not use KServiceTypeTrader or KSyCoCa. Currently, no caching is done, this should/will be implemented separately on top of this patch.
> 
> The implementation is losely based on KServiceTypeTrader, meant for easy porting.
> 
> Code, along with other needed patches can be found in kdelibs[sebas/pluginlocator].
> 
> 
> Diffs
> -----
> 
>   staging/kservice/src/plugin/kplugintrader.h PRE-CREATION 
>   staging/kservice/src/plugin/kplugintrader.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/112680/diff/
> 
> 
> Testing
> -------
> 
> - A whole bunch of newly written unit tests pass, to be submitted separately
> - Ported Plasma::PluginLoader's dataengine handling, it works without apparent regressions
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130912/95fcd808/attachment-0001.html>