<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="http://git.reviewboard.kde.org/r/111686/">http://git.reviewboard.kde.org/r/111686/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 25th, 2013, 4:33 p.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/111686/diff/1/?file=173289#file173289line235" style="color: black; font-weight: bold; text-decoration: underline;">staging/kservice/src/plugin/kpluginloader.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">KPluginFactory *KPluginLoader::factory()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">235</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">args</span> <span class="o"><<</span> <span class="n">metaData</span><span class="p">().</span><span class="n">toVariantMap</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This relates to my comment in the other review. If what we have is a list with always just one parameter (which is a map), why not pass the map directly in setArgs, rather than a list of maps with one map in the list?
IOW: setArgs(QVariantMap).</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">For this to work, we also need the macro to include the json file. That means that we have to introduce a new DECLARATION (temp-)macro to allow passing in a .json file. I was planning to put this ahead, and get this merged first, but as we need a coherent solution for the whole chain, a new version of the patch will add this macro as well, and then we can synchronize it also with the KPluginInfo ctor patch. I'm working on these changes right now, then I'll look into how the layout of the arguments passed around should look like.</pre>
<br />
<p>- Sebastian</p>
<br />
<p>On July 25th, 2013, 4:10 p.m. UTC, Sebastian Kügler wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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 KDE Frameworks and David Faure.</div>
<div>By Sebastian Kügler.</div>
<p style="color: grey;"><i>Updated July 25, 2013, 4:10 p.m.</i></p>
<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;">Make K_EXPORT_PLUGIN work with Qt's new plugin system
This patch changes the K_EXPORT_MACRO and the class it generates to be compatible with Qt's new plugin / metadata system. It basically replaces the old macros around q_plugin_instance with the new ones, using Q_INTERFACES. There's also a setter for the args, which are used to pass metadata into the plugin.
Otherwise, this is the minimal change, to make old plugin factories work atop the new framework.
This change is source-compatible, but the right .moc file when this macro is used from the .cpp file.</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;">Loaded plugins using KService, KPluginLoader, QPluginLoader and Plasma::PluginLoader, all work as expected.</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>staging/kservice/src/plugin/kexportplugin.h <span style="color: grey">(cc5d58b)</span></li>
<li>staging/kservice/src/plugin/kpluginfactory.h <span style="color: grey">(a5ea21b)</span></li>
<li>staging/kservice/src/plugin/kpluginfactory.cpp <span style="color: grey">(6bd2350)</span></li>
<li>staging/kservice/src/plugin/kpluginfactory_p.h <span style="color: grey">(09fcfe4)</span></li>
<li>staging/kservice/src/plugin/kpluginloader.cpp <span style="color: grey">(945c75b)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111686/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>