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
