<table><tr><td style="">dfaure requested changes to this revision.<br />dfaure 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/D8336" 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/D8336#inline-41030" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kjobtrackerinterface.h:88</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 style="color: #74777d"> */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">virtual</span> <span style="color: #aa4000">void</span> <span style="color: #004012">unregisterJob</span><span class="p">(</span><span class="n">KJob</span> <span style="color: #aa2211">*</span><span class="n">job</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">virtual</span> <span style="color: #aa4000">void</span> <span style="color: #004012">unregisterJob</span><span class="p">(</span><span class="n">KJob</span> <span style="color: #aa2211">*</span><span class="n">job</span><span class="p">);<span class="bright"></span></span><span class="bright"> </span><span style="color: #74777d"><span class="bright">// TODO KF6: should it become protected?</span></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">We should decide now, don't leave a question mark in a TODO for KF6. Typically when the time comes to actually make the change, we won't remember in details why this is there and it'll be even harder to decide about it.</p>
<p style="padding: 0; margin: 8px;">I saw the same with unclear KDE4 TODOs, and then unclear KF5 TODOs.... and probably the same before that too ;)</p>
<p style="padding: 0; margin: 8px;"><a href="https://lxr.kde.org/source/extragear/sysadmin/apper/apperd/TransactionWatcher.cpp" class="remarkup-link" target="_blank" rel="noreferrer">https://lxr.kde.org/source/extragear/sysadmin/apper/apperd/TransactionWatcher.cpp</a> looks like a piece of code that would be broken by this being made protected, if I'm not mistaken.<br />
But then again, it might be broken code in the first place, in which case we can still make the change... please investigate.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R244 KCoreAddons</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D8336" rel="noreferrer">https://phabricator.kde.org/D8336</a></div></div><br /><div><strong>To: </strong>elvisangelaccio, kossebau, dfaure<br /><strong>Cc: </strong>apol, Frameworks<br /></div>