<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/104969/">http://git.reviewboard.kde.org/r/104969/</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;">Looks like a sensible change, otherwise. If you've fixed the issues aseigo notes, please go ahead and merge into master.
In the same part, there's an interaction problem, however:
Assumption: installing an app is quite a common thing during development (correct me if you think otherwise)
Right now, there is no button to trigger installation with the currently selected mode, so one has to switch the combo to empty, then to the preferred installation method. A button "Install!" would fix that.
Also, doing the installtion when the combobox changes is quite uncommon, as it is not explicit that the installation actually already happens when you choose something. This should move into a button, which can conveniently go right next to the combo, and then the combo changed to not install on indexChanged. Also, it would make sense to add an action andshortcut for installation to the toolbar above.
This is not an issue with this patch, just how we should improve that part of the UI. I used it for a development task yesterday, and this installatio workflow issue struck me as overly complex and "have to learn it" rather than "understand it without thinking".
</pre>
<br />
<p>- Sebastian</p>
<br />
<p>On May 16th, 2012, 6:38 p.m., Giorgos Tsiapaliwkas 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 Giorgos Tsiapaliwkas.</div>
<p style="color: grey;"><i>Updated May 16, 2012, 6:38 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;">Hello,
those are some issues which plasmate's publisher has
problem 1: the publisher's combobox wasn't aware for the right slot. When currentIndex was emitted both slots(doPlasmaPkg and doCMake was called.) I fixed that.
problem 2: publisher's cmake process was trying to install a projectName.plasmoid file and not projectPath/CMakeLists.txt. I fixed that.
problem 3: when the cmake slot is called, cmake creates the known temporary files in directory like ~/.kde4/share/apps/plasmate/projectName. I haven't fixed that.
How can I change the current directory with Qt?</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;">WIP
</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>publisher/publisher.h <span style="color: grey">(6eba693)</span></li>
<li>publisher/publisher.cpp <span style="color: grey">(3fcd268)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104969/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>