-----------------------------------------------------------
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
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to