<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/117112/">https://git.reviewboard.kde.org/r/117112/</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;">I think that's very sensible. I'd like David to get a chance to look at it though.</pre>
<br />
<p>- Kevin Ottens</p>
<br />
<p>On March 27th, 2014, 12:37 p.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 and David Faure.</div>
<div>By Alex Merry.</div>
<p style="color: grey;"><i>Updated March 27, 2014, 12:37 p.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;">Make KPLuginLoader encapsulate QPluginLoader, rather than inheriting
KPluginLoader was overloading various non-virtual methods, which is a
dangerous business. It probably wouldn't have resulted in any bugs that
were *too* serious, other than some possibly mismatched plugin names if
you managed to call setFileName, but it's best to do these things
properly.
While SIC, this should not affect anyone who was using KPluginLoader sensibly, ie: it should only be an issue if you did
QPluginLoader *loader = new KPluginLoader("foo")</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. I intend to extend the KPluginLoader test, but I thought I'd get the review up since time is tight before beta1.</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>src/plugin/kpluginloader.h <span style="color: grey">(56b3e8b9fd8e143fed16be1575c6bf2e5c979285)</span></li>
<li>src/plugin/kpluginloader.cpp <span style="color: grey">(6d831ac9681e9b26e4644a9e4771eb24c545dac7)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117112/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>