Review Request 118626: KDEConnect-KDE: 16 commits pack.

[Albert Vaca Cintora]

> On June 9, 2014, 5:21 p.m., Aleix Pol Gonzalez wrote:
> > I'm sorry if I missed some discussion, but why is the change from Device -> IDevice?
> > 
> > This patch is changing many unrelated things, it's a bit misleading.
> 
> Alexandr Akulich wrote:
>     I think you missed description. :)
>     
>     As you can see at http://quickgit.kde.org/?p=scratch%2Fakulichalexandr%2Fkdeconnect-kde.git,
>     in commit "Plugins ported to IDevice API. Removed Device/PluginLoader dependency." (next after
>     "Introduced IDevice interface to be used in plugins"), it is used to avoid useless
>     pluginloader.cpp (and, by the way, device.cpp) object linking into each plugin.
> 
> Aleix Pol Gonzalez wrote:
>     Oh I see! They were building this inside the plugin, that looks pretty bad!
>     
>     Ok, +1 here.
> 
> Alexandr Akulich wrote:
>     Thank you! :)

I don't know which is the correct way to do this, do you mind if we discuss it here? The only option I see besides compiling the Device class in every plugin (which, I agree, is a bad idea) or making this "virtual" trick (that, however, adds some overhead in every call) would be to create a "libkdeconnectplugin" that contains the classes that the plugins need so they can link to it. What do you think is the best way to accomplish it? No solutions seems to be perfect :/


- Albert


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


On June 9, 2014, 3:59 p.m., Alexandr Akulich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118626/
> -----------------------------------------------------------
> 
> (Updated June 9, 2014, 3:59 p.m.)
> 
> 
> Review request for kdeconnect and Albert Vaca Cintora.
> 
> 
> Repository: kdeconnect-kde
> 
> 
> Description
> -------
> 
> Easier to view at http://quickgit.kde.org/?p=scratch%2Fakulichalexandr%2Fkdeconnect-kde.git (master branch)
> 
> c67ba08 libkdeconnect: Fixed some style issues.
> 7da4d5b Added some more const declaration.
> c8e5f73 NotificationsModel: isAnyDimissable() marked as invokable method.
> 0657dbb libkdeconnect: KDECONNECT_EXPORT turned to be import declaration outside of lib.
> 8f27d03 kcm: Added DevicesModel declaration to move it's definition to implementation.
> 62c760a Get rid "libkdeconnect" path from includes in lib headers.
> e818d29 Added headers installation.
> 185e3ba DevicesModel: Updated StatusReachable value.
> ab8606e libkdeconnect: CMake files reworked to follow packaging guilde recommendations.
> 3eb44b0 libkdeconnect: Implemented CMake package configuration files.
> a4b9ca9 KDED/PluginLoader: Implemented standalone plugins support.
> 2f197f3 KDED/PluginLoader: Fixed multiply package-type support.
> 8e1172a KDED/Daemon: Implemented standalone plugins loading on construction.
> 725f071 KDED/Default_Args: Added template for QStringList.
> e25ea19 KDED: Introduced IDevice interface to be used in plugins.
> 2a67291 KDED: Plugins ported to IDevice API. Removed Device/PluginLoader dependency.
> 
> 
> Diffs
> -----
> 
>   kcm/CMakeLists.txt 505359b 
>   cli/CMakeLists.txt 6bc525e 
>   kcm/kcm.h 072c544 
>   kcm/kcm.cpp daaac3a 
>   kded/CMakeLists.txt e2dcb21 
>   kded/daemon.h 686c393 
>   kded/daemon.cpp fb1c166 
>   kded/default_args.h ff1b7f7 
>   kded/device.h 3f97b19 
>   kded/device.cpp ee13dcc 
>   kded/idevice.h PRE-CREATION 
>   kded/plugins/battery/CMakeLists.txt d623141 
>   kded/plugins/clipboard/CMakeLists.txt 426cfef 
>   kded/plugins/clipboard/clipboardplugin.h 9a7cab4 
>   kded/plugins/clipboard/clipboardplugin.cpp 9acfa76 
>   kded/plugins/kdeconnectplugin.h e8ff017 
>   kded/plugins/kdeconnectplugin.cpp 4971c49 
>   kded/plugins/mpriscontrol/CMakeLists.txt dd2a2a3 
>   kded/plugins/notifications/CMakeLists.txt a6ed32a 
>   kded/plugins/notifications/notificationsdbusinterface.h 9865ae7 
>   kded/plugins/notifications/notificationsdbusinterface.cpp c56a3d8 
>   kded/plugins/pluginloader.h 1a03259 
>   kded/plugins/pluginloader.cpp 737edae 
>   kded/plugins/sftp/CMakeLists.txt fa8f082 
>   kded/plugins/sftp/mounter.h 91b6bad 
>   kded/plugins/share/CMakeLists.txt ae4e18f 
>   kio/CMakeLists.txt 0d58ced 
>   libkdeconnect/CMakeLists.txt b65da06 
>   libkdeconnect/KDEConnect/DevicesModel PRE-CREATION 
>   libkdeconnect/KDEConnect/NotificationsModel PRE-CREATION 
>   libkdeconnect/KDEConnectConfig.cmake.in PRE-CREATION 
>   libkdeconnect/dbusinterfaces.h c5b4905 
>   libkdeconnect/devicesmodel.h a84c84d 
>   libkdeconnect/devicesmodel.cpp 7b7a749 
>   libkdeconnect/kdeconnect_export.h 56c2459 
>   libkdeconnect/notificationsmodel.h a79c063 
>   libkdeconnect/notificationsmodel.cpp 46dc8ee 
>   plasmoid/declarativeplugin/CMakeLists.txt 4919d47 
> 
> Diff: https://git.reviewboard.kde.org/r/118626/diff/
> 
> 
> Testing
> -------
> 
> Done.
> 
> 
> Thanks,
> 
> Alexandr Akulich
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20140610/c80c16ed/attachment-0001.html>