<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/106500/">http://git.reviewboard.kde.org/r/106500/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 19th, 2012, 5:11 p.m., <b>Oswald Buddenhagen</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;">every single change of this patch is a regression</pre>
</blockquote>
<p>On September 19th, 2012, 7:20 p.m., <b>David Faure</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;">Very encouraging, as always. Care to give us more details?</pre>
</blockquote>
<p>On September 20th, 2012, 9:09 p.m., <b>Oswald Buddenhagen</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;">the reasons for kprocess' existence didn't go away, so every change away from it is by definition a regression, either in functionality or maintainablity. and the consistent replacement of setOutputChannelMode() with setReadChannel() is so bizarre that one has to wonder what i wrote the apidoc for in the first place.
in case somebody is interested, i still have the (rather unfinished) qprocess patches i wrote five years ago to address (some of) the issues upstream. it would be qt 5.1 material, obviously.</pre>
</blockquote>
<p>On September 20th, 2012, 9:38 p.m., <b>David Faure</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;">That reasoning assumes that we need the additional-features-in-KProcess everywhere KProcess is used, which is just not the case.
See commits ed71a84ca2178865d947169a8f35a025c708a2ef and 66fda6d3faafeadac82eae6b8308787b3fe96911 which already ported some KProcess to QProcess, and which did not introduce regressions (most are in unittests, which still pass).
You're right about setOutputChannelMode, though -- apparently the proper replacement is QProcess::setProcessChannelMode.
Still I don't see why we can't use QProcess in its current form... like everything it could be improved, but surely all other Qt apps manage just fine?</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;">ed71a84ca2178 is arguably broken.
yes. a rather suboptimal replacement. before you ask, search the core-devel archive shortly before the time the new kprocess was added.
other apps managed with motif just fine, too. doesn't mean the users liked the outcome.</pre>
<br />
<p>- Oswald</p>
<br />
<p>On September 18th, 2012, 11:12 p.m., Kevin Funk 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 KDE Frameworks.</div>
<div>By Kevin Funk.</div>
<p style="color: grey;"><i>Updated Sept. 18, 2012, 11:12 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;">Port some uses of KProcess to QProcess</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>kdeui/dialogs/kedittoolbar.cpp <span style="color: grey">(0582934b0bf8cb7160cb48b4c8151c81b35277f0)</span></li>
<li>kinit/klauncher.cpp <span style="color: grey">(855e56041be5a5b76b9a7e9d0597ac7ad485682e)</span></li>
<li>kio/kfile/kfilemetadatareader.cpp <span style="color: grey">(88cadaa2edf1b1de24c0e91576cca368db41f470)</span></li>
<li>kio/kio/krun.h <span style="color: grey">(7bfe66b59f1deffc37d3ceae999fb929e453fd31)</span></li>
<li>kio/kio/krun.cpp <span style="color: grey">(031dbc1dfef685729038b4a59cbeacd34d448ed2)</span></li>
<li>kio/kio/krun_p.h <span style="color: grey">(0ad15c8434599ccabcd649f251aa622d4fb0b0f7)</span></li>
<li>staging/kwidgets/autotests/kglobalsettingstest.cpp <span style="color: grey">(4426fee08427499c777cc7fc94e4b1345c790ac2)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106500/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>