<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/102175/">http://git.reviewboard.kde.org/r/102175/</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 don't like the idea of this being public API, at least not until we know we need it to be.
i also recommend changing the name of the class from EngineInstaller to ComponentInstaller, since it seems more generic than just "Engines" (which itself is ambiguous due to ScriptEngines and DataEngines).</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="http://git.reviewboard.kde.org/r/102175/diff/1/?file=30596#file30596line53" style="color: black; font-weight: bold; text-decoration: underline;">plasma/CMakeLists.txt</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; "></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">53</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">endif(NOT PLASMA_NO_QTDBUS)</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;">mismatch with if</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="http://git.reviewboard.kde.org/r/102175/diff/1/?file=30599#file30599line78" style="color: black; font-weight: bold; text-decoration: underline;">plasma/engineinstaller.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="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void EngineInstaller::installMissingEngine(const QString &type,</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">78</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">d</span><span class="o">-></span><span class="n">alreadyPrompted</span><span class="p">.</span><span class="n">insert</span><span class="p">(</span><span class="n">searchString</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;">so if !force, it shouldn't be added to alreadyPrompted?
it may also be worthwhile to remove items from alreadyPrompted once there is success (thus freeing up that memory), assuming packagekit returns this information.</pre>
</div>
<br />
<p>- Aaron J.</p>
<br />
<p>On August 2nd, 2011, 2:57 a.m., Kevin Kofler wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Kevin Kofler.</div>
<p style="color: grey;"><i>Updated Aug. 2, 2011, 2:57 a.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;">This is part of my GSoC 2011 work. Expect more Plasma patches related to this in the next few days.
In particular, the currently unused "force" feature of the new API is intended to be used if the user just installed a widget which explicitly requires a given engine. (By default, prompts are not repeated within a session to prevent flooding the user.)
The implementation is based on PackageKit. It requires PackageKit >= 0.6.16 and either Apper trunk or the backported patch from http://pkgs.fedoraproject.org/gitweb/?p=kpackagekit.git;a=blob;f=kpackagekit06-plasma.patch;h=208427aa6cc072164b6a9ba48a30a954657ef892;hb=HEAD to have any effect. If the requirements are not met, no change will be noticeable at all, because the asynchronous call will simply fail and any errors are (intentionally) discarded. (We don't want to annoy the user with a pointless error dialog if he/she doesn't have PackageKit installed or his/her version is too old.)
PackageKit will also only actually find the relevant packages if the distribution is using RPM and yum (at this time) and if the RPMs in the repository have been built with my Provides generator. But that shouldn't be Plasma's problem. Any work in that area needs to happen on the PackageKit or distro side. Support for GStreamer plugins has been implemented in other PackageKit backends too, so it should also be doable for Plasma engines. And if a distro wants to use something entirely different from PackageKit, that's also possible, this is what the abstract EngineInstaller API is for.
The API is public because it might be useful outside of libplasma, and it is quite simple and generic, thus keeping BC should not be a problem.</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;">Verified that it compiles without errors and that it successfully prompts for a missing Python script engine on Fedora 15. (The patch is against master (4.8), but applies without changes to the kdelibs 4.6.5 in Fedora 15, which is how I tested it.)</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>includes/CMakeLists.txt <span style="color: grey">(a967a92)</span></li>
<li>includes/Plasma/EngineInstaller <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasma/CMakeLists.txt <span style="color: grey">(ef411df)</span></li>
<li>plasma/dataenginemanager.cpp <span style="color: grey">(988fe76)</span></li>
<li>plasma/engineinstaller.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasma/engineinstaller.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasma/scripting/scriptengine.cpp <span style="color: grey">(fb8cd1a)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102175/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>