<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/118626/">https://git.reviewboard.kde.org/r/118626/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 9th, 2014, 5:21 p.m. CEST, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On June 9th, 2014, 8:39 p.m. CEST, <b>Alexandr Akulich</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On June 10th, 2014, 5:52 p.m. CEST, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Oh I see! They were building this inside the plugin, that looks pretty bad!
Ok, +1 here.</pre>
</blockquote>
<p>On June 10th, 2014, 6:09 p.m. CEST, <b>Alexandr Akulich</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thank you! :)</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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 :/</pre>
<br />
<p>- Albert</p>
<br />
<p>On June 9th, 2014, 3:59 p.m. CEST, Alexandr Akulich wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdeconnect and Albert Vaca Cintora.</div>
<div>By Alexandr Akulich.</div>
<p style="color: grey;"><i>Updated June 9, 2014, 3:59 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdeconnect-kde
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Done.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kcm/CMakeLists.txt <span style="color: grey">(505359b)</span></li>
<li>cli/CMakeLists.txt <span style="color: grey">(6bc525e)</span></li>
<li>kcm/kcm.h <span style="color: grey">(072c544)</span></li>
<li>kcm/kcm.cpp <span style="color: grey">(daaac3a)</span></li>
<li>kded/CMakeLists.txt <span style="color: grey">(e2dcb21)</span></li>
<li>kded/daemon.h <span style="color: grey">(686c393)</span></li>
<li>kded/daemon.cpp <span style="color: grey">(fb1c166)</span></li>
<li>kded/default_args.h <span style="color: grey">(ff1b7f7)</span></li>
<li>kded/device.h <span style="color: grey">(3f97b19)</span></li>
<li>kded/device.cpp <span style="color: grey">(ee13dcc)</span></li>
<li>kded/idevice.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kded/plugins/battery/CMakeLists.txt <span style="color: grey">(d623141)</span></li>
<li>kded/plugins/clipboard/CMakeLists.txt <span style="color: grey">(426cfef)</span></li>
<li>kded/plugins/clipboard/clipboardplugin.h <span style="color: grey">(9a7cab4)</span></li>
<li>kded/plugins/clipboard/clipboardplugin.cpp <span style="color: grey">(9acfa76)</span></li>
<li>kded/plugins/kdeconnectplugin.h <span style="color: grey">(e8ff017)</span></li>
<li>kded/plugins/kdeconnectplugin.cpp <span style="color: grey">(4971c49)</span></li>
<li>kded/plugins/mpriscontrol/CMakeLists.txt <span style="color: grey">(dd2a2a3)</span></li>
<li>kded/plugins/notifications/CMakeLists.txt <span style="color: grey">(a6ed32a)</span></li>
<li>kded/plugins/notifications/notificationsdbusinterface.h <span style="color: grey">(9865ae7)</span></li>
<li>kded/plugins/notifications/notificationsdbusinterface.cpp <span style="color: grey">(c56a3d8)</span></li>
<li>kded/plugins/pluginloader.h <span style="color: grey">(1a03259)</span></li>
<li>kded/plugins/pluginloader.cpp <span style="color: grey">(737edae)</span></li>
<li>kded/plugins/sftp/CMakeLists.txt <span style="color: grey">(fa8f082)</span></li>
<li>kded/plugins/sftp/mounter.h <span style="color: grey">(91b6bad)</span></li>
<li>kded/plugins/share/CMakeLists.txt <span style="color: grey">(ae4e18f)</span></li>
<li>kio/CMakeLists.txt <span style="color: grey">(0d58ced)</span></li>
<li>libkdeconnect/CMakeLists.txt <span style="color: grey">(b65da06)</span></li>
<li>libkdeconnect/KDEConnect/DevicesModel <span style="color: grey">(PRE-CREATION)</span></li>
<li>libkdeconnect/KDEConnect/NotificationsModel <span style="color: grey">(PRE-CREATION)</span></li>
<li>libkdeconnect/KDEConnectConfig.cmake.in <span style="color: grey">(PRE-CREATION)</span></li>
<li>libkdeconnect/dbusinterfaces.h <span style="color: grey">(c5b4905)</span></li>
<li>libkdeconnect/devicesmodel.h <span style="color: grey">(a84c84d)</span></li>
<li>libkdeconnect/devicesmodel.cpp <span style="color: grey">(7b7a749)</span></li>
<li>libkdeconnect/kdeconnect_export.h <span style="color: grey">(56c2459)</span></li>
<li>libkdeconnect/notificationsmodel.h <span style="color: grey">(a79c063)</span></li>
<li>libkdeconnect/notificationsmodel.cpp <span style="color: grey">(46dc8ee)</span></li>
<li>plasmoid/declarativeplugin/CMakeLists.txt <span style="color: grey">(4919d47)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118626/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>