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