D29451: KNS: Do not mark entry as installed if install script failed

Dan Leinir Turthra Jensen noreply at phabricator.kde.org
Wed May 6 14:00:45 BST 2020


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200506/7b68b432/attachment.html>


More information about the Kde-frameworks-devel mailing list