<table><tr><td style="">ervin requested changes to this revision.<br />ervin 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/D9015" rel="noreferrer">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/D9015#inline-40563" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kfileplacesview.cpp:811-813</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; background: rgba(251, 175, 175, .7);">    <span style="color: #aa4000">if</span> <span class="p">(<span class="bright"></span></span><span class="bright"></span><span class="n"><span class="bright">emptyTrash</span></span> <span style="color: #aa2211">!=</span> <span style="color: #aa4000">nullptr<span class="bright"></span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">&&</span></span><span class="bright"> </span><span class="n"><span class="bright">result</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">==</span></span><span class="bright"> </span><span class="n"><span class="bright">emptyTrash</span></span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #aa4000">if</span> <span class="p">(<span class="bright"></span></span><span class="bright"></span><span class="n"><span class="bright">result</span></span> <span style="color: #aa2211">!=</span> <span style="color: #aa4000">nullptr</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">        <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">result</span> <span style="color: #aa2211">==</span> <span class="n">emptyTrash</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;">This whole change on the if structure there is unrelated to the rest of the patch so please remove it. Beside this seems to change the semantic, we never tested for result being nullptr or not before, we tested for the various actions to be there, you changed that logic completely that smells dangerous to me.</p></div></div><br /><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/D9015#inline-40562" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kfileplacesview.h:111-112</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">Q_PRIVATE_SLOT</span><span class="p">(</span><span class="n">d</span><span class="p">,</span> <span style="color: #aa4000">void</span> <span class="n">_k_triggerDevicePolling</span><span class="p">())</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">Q_PRIVATE_SLOT</span><span class="p">(</span><span class="n">d</span><span class="p">,</span> <span style="color: #aa4000">void</span> <span class="n">triggerItemAppearingAnimation</span><span class="p">())</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">Q_PRIVATE_SLOT</span><span class="p">(</span><span class="n">d</span><span class="p">,</span> <span style="color: #aa4000">void</span> <span class="n">triggerItemDisappearingAnimation</span><span class="p">())</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">They are never used as slots AFAICT so this should go away I think.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D9015" rel="noreferrer">https://phabricator.kde.org/D9015</a></div></div><br /><div><strong>To: </strong>franckarrecot, ervin, renatoo, mlaurent, ngraham<br /><strong>Cc: </strong>Frameworks<br /></div>