D20209: Add support for KBookmarkOwner to communicate if it has tabs open

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Thanks for this contribution.
  
  What you did with member variables in the unittest, is generally done with data-driven testing instead -- which I find preferable, better locality and readability. See "Data Driven Testing" in Qt Assistant.
  One issue with member vars is that one test method might affect the next one. This is why it's much better to not have any, in test classes.

INLINE COMMENTS

> kbookmarkowner.h:120
> +     */
> +    virtual bool tabsOpen() const
> +    {

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.

Bigger problem: this is a public class. You CANNOT add a new virtual method, that is binary incompatible.
You have to do this with a setter and a getter (and putting the member variable behind the d pointer).

REPOSITORY
  R294 KBookmarks

REVISION DETAIL
  https://phabricator.kde.org/D20209

To: hallas, #frameworks, ngraham, cfeck, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190413/3968333c/attachment.html>