<table><tr><td style="">alex added inline comments.
</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/D29123">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/D29123#inline-168778">View Inline</a><span style="color: #4b4d51; font-weight: bold;">leinir</span> wrote in <span style="color: #4b4d51; font-weight: bold;">installation.h:124</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Hmm... i find myself wondering what effect this has on BIC... something tells me it might not be so great... Specifically, <a href="https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B#The_Do.27s_and_Don.27ts" class="remarkup-link" target="_blank" rel="noreferrer">https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B#The_Do.27s_and_Don.27ts</a> says that you cannot change the signature of existing functions of any type... However, it feels more like a problem with the documentation - the idea is that the entries passed into functions aren't changed (and this really should probably be a const reference, but for some reason it isn't, and again we can't change that for bic-iness), and so really what /should/ happen is that rather than saying "The entry instance will be updated" and so on, what it should be doing is say "If the entry is successfully uninstalled, listening to signalEntryChanged(const KNSCore::EntryInternal &) for an entry equal to the one you have passed in will allow you to detect the result of calling the function".</p>

<p style="padding: 0; margin: 8px;">That's what it's already doing, and how it is used in places which call the function, so probably makes sense to fix that.</p>

<p style="padding: 0; margin: 8px;">In the longer term (think KF6), i would also quite like all the functionality here to end up entirely asynchronous.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I told myself yesterday that I am going to have a look at this patch again^^<br />
And you are absolutely right :-)</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/D29123">https://phabricator.kde.org/D29123</a></div></div><br /><div><strong>To: </strong>alex, KNewStuff, meven, ngraham, leinir<br /><strong>Cc: </strong>leinir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns<br /></div>