Review Request: Port some uses of KProcess to QProcess

[David Faure]

> On Sept. 19, 2012, 5:11 p.m., Oswald Buddenhagen wrote:
> > every single change of this patch is a regression
> 
> David Faure wrote:
>     Very encouraging, as always. Care to give us more details?
> 
> Oswald Buddenhagen wrote:
>     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.

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?


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106500/#review19174
-----------------------------------------------------------


On Sept. 18, 2012, 11:12 p.m., Kevin Funk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106500/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2012, 11:12 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> Port some uses of KProcess to QProcess
> 
> 
> Diffs
> -----
> 
>   kdeui/dialogs/kedittoolbar.cpp 0582934b0bf8cb7160cb48b4c8151c81b35277f0 
>   kinit/klauncher.cpp 855e56041be5a5b76b9a7e9d0597ac7ad485682e 
>   kio/kfile/kfilemetadatareader.cpp 88cadaa2edf1b1de24c0e91576cca368db41f470 
>   kio/kio/krun.h 7bfe66b59f1deffc37d3ceae999fb929e453fd31 
>   kio/kio/krun.cpp 031dbc1dfef685729038b4a59cbeacd34d448ed2 
>   kio/kio/krun_p.h 0ad15c8434599ccabcd649f251aa622d4fb0b0f7 
>   staging/kwidgets/autotests/kglobalsettingstest.cpp 4426fee08427499c777cc7fc94e4b1345c790ac2 
> 
> Diff: http://git.reviewboard.kde.org/r/106500/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin Funk
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20120920/c0a3d962/attachment.html>