Review Request: Port some uses of KProcess to QProcess

Oswald Buddenhagen ossi at kde.org
Thu Sep 20 22:02:03 UTC 2012



> 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.
> 
> David Faure wrote:
>     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?

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.


- Oswald


-----------------------------------------------------------
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/a50e81de/attachment.html>


More information about the Kde-frameworks-devel mailing list