leinir requested changes to this revision.
leinir added inline comments.
This revision now requires changes to proceed.
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);
> + connect(p, static_cast<void(QProcess::*)(int,
> QProcess::ExitStatus)>(&QProcess::finished),
> + [entry, installationFinished, this] (int exitCode,
> QProcess::ExitStatus) {
Remember to give connect an object context for the slot (i did not realise
until recently what leaving that out means, but it turns out to be potentially
quite bad and crashy in difficult to track ways - in short, if our this
instance is destroyed (say, the user quits the application) while an
installation is ongoing, this would crash when attempting to emit or call
installationFinished - it /should be fine, as i said, in most cases, as
installation's a long lifetime object, but also just good style to add that
context - see
https://wiki.qt.io/New_Signal_Slot_Syntax#New:_connecting_to_simple_function
for a detailed explanation, but in short, just add `this,` before the lambda
and you're good to go :) )
REPOSITORY
R304 KNewStuff
REVISION DETAIL
https://phabricator.kde.org/D29451
To: alex, #knewstuff, ngraham, leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
