<table><tr><td style="">hallas 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/D20209">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/D20209#inline-115306">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kbookmarkowner.h:120</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">One day we'll probably wonder how many tabs are actually open, to avoid saying "Bookmark Tabs As Folder" if there's only one tab. So I'd prefer for that and make this an int rather than a bool.</p>

<p style="padding: 0; margin: 8px;">Bigger problem: this is a public class. You CANNOT add a new virtual method, that is binary incompatible.<br />
You have to do this with a setter and a getter (and putting the member variable behind the d pointer).</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Good point (both of them)</p>

<p style="padding: 0; margin: 8px;">What are our binary compatibility promises? Is <em>every</em> version of frameworks always binary backwards compatible? Or do we have a window every now and then where we can make source compatible changes but not binary compatible? I don't mind changing this to be using settings and getters, but that just makes this API stand out from the rest. All the other APIs in the class are virtual functions which should be overwritten by the actual owner. I know this does not argue against our binary compatible promise though :D</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R294 KBookmarks</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D20209">https://phabricator.kde.org/D20209</a></div></div><br /><div><strong>To: </strong>hallas, Frameworks, ngraham, cfeck, dfaure<br /><strong>Cc: </strong>kde-frameworks-devel, michaelh, ngraham, bruns<br /></div>