<table><tr><td style="">leinir requested changes to this revision.<br />leinir added inline comments.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D29451">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D29451#inline-168944">View Inline</a><span style="color: #4b4d51; font-weight: bold;">installation.cpp:355</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; ">            <span class="n">QProcess</span><span style="color: #aa2211">*</span> <span class="n">p</span> <span style="color: #aa2211">=</span> <span class="n">runPostInstallationCommand</span><span class="p">(</span><span class="n">installedFiles</span><span class="p">.</span><span class="n">size</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span style="color: #601200">1</span> <span style="color: #aa2211">?</span> <span class="n">installedFiles</span><span class="p">.</span><span class="n">first</span><span class="p">()</span> <span style="color: #aa2211">:</span> <span class="n">targetPath</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">            <span class="n">connect</span><span class="p">(</span><span class="n">p</span><span class="p">,</span> <span style="color: #aa4000">static_cast</span><span style="color: #aa2211"><</span><span style="color: #aa4000">void</span><span class="p">(</span><span class="n">QProcess</span><span style="color: #aa2211">::*</span><span class="p">)(</span><span style="color: #aa4000">int</span><span class="p">,</span> <span class="n">QProcess</span><span style="color: #aa2211">::</span><span class="n">ExitStatus</span><span class="p">)</span><span style="color: #aa2211">></span><span class="p">(</span><span style="color: #aa2211">&</span><span class="n">QProcess</span><span style="color: #aa2211">::</span><span class="n">finished</span><span class="p">),<span class="bright"></span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">this</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">installationFinished</span></span><span class="bright"></span><span class="p"><span class="bright">);</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">            <span class="n">connect</span><span class="p">(</span><span class="n">p</span><span class="p">,</span> <span style="color: #aa4000">static_cast</span><span style="color: #aa2211"><</span><span style="color: #aa4000">void</span><span class="p">(</span><span class="n">QProcess</span><span style="color: #aa2211">::*</span><span class="p">)(</span><span style="color: #aa4000">int</span><span class="p">,</span> <span class="n">QProcess</span><span style="color: #aa2211">::</span><span class="n">ExitStatus</span><span class="p">)</span><span style="color: #aa2211">></span><span class="p">(</span><span style="color: #aa2211">&</span><span class="n">QProcess</span><span style="color: #aa2211">::</span><span class="n">finished</span><span class="p">),</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                    <span class="p">[</span><span class="n">entry</span><span class="p">,</span> <span class="n">installationFinished</span><span class="p">,</span> <span style="color: #aa4000">this</span><span class="p">]</span> <span class="p">(</span><span style="color: #aa4000">int</span> <span class="n">exitCode</span><span class="p">,</span> <span class="n">QProcess</span><span style="color: #aa2211">::</span><span class="n">ExitStatus</span><span class="p">)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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 <a href="https://wiki.qt.io/New_Signal_Slot_Syntax#New:_connecting_to_simple_function" class="remarkup-link" target="_blank" rel="noreferrer">https://wiki.qt.io/New_Signal_Slot_Syntax#New:_connecting_to_simple_function</a> for a detailed explanation, but in short, just add <tt style="background: #ebebeb; font-size: 13px;">this,</tt> before the lambda and you're good to go :) )</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R304 KNewStuff</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D29451">https://phabricator.kde.org/D29451</a></div></div><br /><div><strong>To: </strong>alex, KNewStuff, ngraham, leinir<br /><strong>Cc: </strong>kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns<br /></div>