kossebau added a comment.
Some API (docs) nitpicks :) INLINE COMMENTS > kbookmarkmenu.cpp:88 > + m_parentMenu(_parentMenu), > + m_parentAddress(QLatin1String("")) //TODO KBookmarkAdress::root > +{ Why the `QLatin1String("")` instead of QString()? Is there a need for a non-null empty string? It#s copied from the other constructor, but should get a comment while at it if that is the case, or just get to be QString() > kbookmarkmenu.h:89 > */ > +#if KBOOKMARKS_ENABLE_DEPRECATED_SINCE(5, 65) > + KBOOKMARKS_DEPRECATED_VERSION(5, 65, "Use overload without > KActionCollection and add actions manually to your actioncollection if > desired") The visibility guard for deprecated methods should start before the API dox , if only for consistency, so please move to front. See https://community.kde.org/Policies/Library_Code_Policy#Deprecation_of_API > kbookmarkmenu.h:99 > + * > + * @param mgr The bookmark manager to use (i.e. for reading and writing) > + * @param owner implementation of the KBookmarkOwner callback interface. -> lowercase "the", cmp. https://community.kde.org/Frameworks/Frameworks_Documentation_Policy#Document_Public_and_Protected_Members > kbookmarkmenu.h:101 > + * @param owner implementation of the KBookmarkOwner callback interface. > + * Note: If you pass a null KBookmarkOwner to the constructor, the > + * openBookmark signal is not emitted, instead QDesktopServices::openUrl > is used to open the bookmark. `Note:` -> `@note` > kbookmarkmenu.h:104 > + * @param parentMenu menu to be filled > + * @param collec parent collection for the KActions. > + * @since 5.65 No such parameter "collec"? > kbookmarkmenu.h:107 > + */ > + KBookmarkMenu(KBookmarkManager *mgr, KBookmarkOwner *owner, QMenu > *parentMenu); > "mgr" -> "manager" while at it, please > kbookmarkmenu.h:144 > + /** > + * Action for adding a bookmark. If you are using KXMLGui add it to your > action collection. > + * @code Official casing is "KXmlGui" :) Also below. > kbookmarkmenu.h:144 > + /** > + * Action for adding a bookmark. If you are using KXMLGui add it to your > action collection. > + * @code This being a method, not a property "Action which is" is not the perfect text, please describe what the method does (getting access to the action owned by the menu). > kbookmarkmenu.h:149 > + * @endcode > + * @since 5.65 > + */ Please add a `@return ...` here and to the other methods, so e.g. IDEs can have better metadata avaialble. > kbookmarkmenu.h:151 > + */ > + QAction *addBookmarkAction(); > + Can be const methods, no? REPOSITORY R294 KBookmarks REVISION DETAIL https://phabricator.kde.org/D25660 To: nicolasfella, #frameworks, dfaure Cc: kossebau, dfaure, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns