D20860: Fix KBoomarkMenuTest when Bookmarks Editor is not installed

David Faure noreply at phabricator.kde.org
Sat Apr 27 14:06:35 BST 2019


dfaure added a comment.


  Thanks for the debugging. Looks good, just minor coding style issues.

INLINE COMMENTS

> kbookmarkmenutest.cpp:42
>  
> +bool hasBookmarkEditorInstalled()
> +{

prepend static (good practice in general so the symbol isn't exported, although in a test program it doesn't make much difference). It becomes a readability difference to me, if I don't see static I wonder which other file wants to call this.

> kbookmarkmenutest.cpp:44
> +{
> +    return !QStandardPaths::findExecutable(QStringLiteral("keditbookmarks")).isEmpty();
> +}

This code could put the result into a (local) static bool variable, to cache the result, but no big deal for a unittest.

> kbookmarkmenutest.cpp:47
> +
> +int actionCountWithoutBookmarkTabsAsFolder()
> +{

prepend static

> kbookmarkmenutest.cpp:52
> +
> +int actionCountWithBookmarkTabsAsFolder()
> +{

prepend static

REPOSITORY
  R294 KBookmarks

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

To: hallas, dfaure, aacid, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190427/601ace0b/attachment.html>


More information about the Kde-frameworks-devel mailing list