leinir requested changes to this revision. leinir added a comment. This revision now requires changes to proceed.
A bit nitpicky, that first one, the second's more serious (i'd like to avoid that in new code), but looks good otherwise :) INLINE COMMENTS > installation.cpp:355 > QProcess* p = runPostInstallationCommand(installedFiles.size() > == 1 ? installedFiles.first() : targetPath); > - connect(p, static_cast<void(QProcess::*)(int, > QProcess::ExitStatus)>(&QProcess::finished), this, installationFinished); > + auto finishedLambda = [=](int exitCode, QProcess::ExitStatus) { > + if (exitCode) { Not really any reason to store it in a variable so much... In the rest of the codebase, unless the same lambda is used in more than one location, it's defined inline in the connect statement. Also, i realise some of the code has the capture everything thing going on, but that's me being silly when i first learned how to use them - if possible, only capture the things you actually need to capture :) REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D29451 To: alex, #knewstuff, ngraham, leinir Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns