<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/117185/">https://git.reviewboard.kde.org/r/117185/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Couple of nitpicks, overall I like where this is going.</pre>
<br />
<div>
<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="https://git.reviewboard.kde.org/r/117185/diff/2/?file=258191#file258191line305" style="color: black; font-weight: bold; text-decoration: underline;">src/plugin/kpluginloader.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<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">304</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">class</span> <span class="n">KSERVICE_EXPORT</span> <span class="n">KPluginName</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Why is this class all inlined?</pre>
</div>
<br />
<div>
<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="https://git.reviewboard.kde.org/r/117185/diff/2/?file=258191#file258191line359" style="color: black; font-weight: bold; text-decoration: underline;">src/plugin/kpluginloader.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<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">358</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span> <span class="n">m_path</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm confused, is that a path or a name?</pre>
</div>
<br />
<p>- Kevin Ottens</p>
<br />
<p>On March 30th, 2014, 11:41 a.m. UTC, Alex Merry 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 KDE Frameworks, David Faure, Kevin Ottens, and Sebastian Kügler.</div>
<div>By Alex Merry.</div>
<p style="color: grey;"><i>Updated March 30, 2014, 11:41 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kservice
</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;">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).</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;">Builds, tests pass.</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>autotests/CMakeLists.txt <span style="color: grey">(dcc4a40bd81f7e17f10219649214a8c9aac5cc60)</span></li>
<li>autotests/jsonplugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/jsonplugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/jsonplugin.json <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/kpluginloadertest.cpp <span style="color: grey">(2b0afb81469f18f93ea43e6ac73df95a60cbd7f3)</span></li>
<li>autotests/kservicetest.h <span style="color: grey">(aa1c691c75c9e5b6903bcbf6e2dc95809ee1ce21)</span></li>
<li>autotests/kservicetest.cpp <span style="color: grey">(796abbcc6313d2cfd69bd213281ea30dfebaa421)</span></li>
<li>autotests/noplugin.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugin/kpluginloader.h <span style="color: grey">(4e10edf0f152b409c6a4031610ff1f2f9771d08d)</span></li>
<li>src/plugin/kpluginloader.cpp <span style="color: grey">(1602c180db5e1a5f0765d95d68f90d2046c9ef2b)</span></li>
<li>src/services/kservice.h <span style="color: grey">(6df389a130587eea732b1d81718181bbd3df4f6d)</span></li>
<li>src/services/kservice.cpp <span style="color: grey">(c897d4c2a08d048e817154101b3a212a098f9768)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117185/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>