<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 />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 22nd, 2012, 11:22 p.m., <b>Sebastian Kügler</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">>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.
I agree. I will update the diff tomorrow with the new button if we all agree with the new button.
>Also, it would make sense to add an action andshortcut for installation to the toolbar above.
I guess it would be a good idea to add an action in the toolbar. What do you think?
>Also, it would make sense to add an action andshortcut for installation to the toolbar above.
Which shortcut do you want?
I would prefer to use the plasmapkg and not cmake.
Everyone is using plasmapkg and it is safer (from the POV of a future bug in cmake) due to the fact the more developers use it.</pre>
<br />
<p>- Giorgos</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>