Review Request 117185: Remove KPluginLoader dependency on KService

[Alex Merry]

> On March 31, 2014, 2:49 p.m., Kevin Ottens wrote:
> > src/plugin/kpluginloader.h, line 305
> > <https://git.reviewboard.kde.org/r/117185/diff/2/?file=258191#file258191line305>
> >
> >     Why is this class all inlined?

Because it's a pretty trivial container class. It was patterned off things like QDBusObjectPath, although that is even simpler (except for the code that checks it is a valid D-Bus object path, which isn't inlined).


> On March 31, 2014, 2:49 p.m., Kevin Ottens wrote:
> > src/plugin/kpluginloader.h, line 359
> > <https://git.reviewboard.kde.org/r/117185/diff/2/?file=258191#file258191line359>
> >
> >     I'm confused, is that a path or a name?

Ah, good point.  That should be m_name.


- Alex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117185/#review54686
-----------------------------------------------------------


On March 30, 2014, 11:41 a.m., Alex Merry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117185/
> -----------------------------------------------------------
> 
> (Updated March 30, 2014, 11:41 a.m.)
> 
> 
> Review request for KDE Frameworks, David Faure, Kevin Ottens, and Sebastian Kügler.
> 
> 
> Repository: kservice
> 
> 
> Description
> -------
> 
> Remove KPluginLoader dependency on KService
> 
> The reason for this is so that KPluginLoader, KPluginFactory and
> kexportplugin.h do not require anything else from the KService
> framework, allowing them to be moved to KCoreAddons.
> 
> A KPluginName class is introduced to act as an intermediary between
> KService and KPluginLoader.  The KPluginLoader(KService) constructor is
> replaced with KPluginLoader(KPluginName), and KService gains a cast
> operator to convert it to a KPluginName.
> 
> Now
>   KService service("blah.desktop");
>   KPluginLoader(service);
> works as before, maintaining source compatibility.
> 
> The reason for the intermediary class (rather than just, say, providing
> a QString() operator to KService) is to make sure this combination is
> the only one that works (and you can't start using KService everywhere a
> QString is expected).
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt dcc4a40bd81f7e17f10219649214a8c9aac5cc60 
>   autotests/jsonplugin.h PRE-CREATION 
>   autotests/jsonplugin.cpp PRE-CREATION 
>   autotests/jsonplugin.json PRE-CREATION 
>   autotests/kpluginloadertest.cpp 2b0afb81469f18f93ea43e6ac73df95a60cbd7f3 
>   autotests/kservicetest.h aa1c691c75c9e5b6903bcbf6e2dc95809ee1ce21 
>   autotests/kservicetest.cpp 796abbcc6313d2cfd69bd213281ea30dfebaa421 
>   autotests/noplugin.desktop PRE-CREATION 
>   src/plugin/kpluginloader.h 4e10edf0f152b409c6a4031610ff1f2f9771d08d 
>   src/plugin/kpluginloader.cpp 1602c180db5e1a5f0765d95d68f90d2046c9ef2b 
>   src/services/kservice.h 6df389a130587eea732b1d81718181bbd3df4f6d 
>   src/services/kservice.cpp c897d4c2a08d048e817154101b3a212a098f9768 
> 
> Diff: https://git.reviewboard.kde.org/r/117185/diff/
> 
> 
> Testing
> -------
> 
> Builds, tests pass.
> 
> 
> Thanks,
> 
> Alex Merry
> 
>

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