graesslin requested changes to this revision. graesslin added a comment. This revision now requires changes to proceed.
First round of review, client api is done. INLINE COMMENTS > registry.h:1275 > + * @param name The name for the removed interface > + * @since 5.25 > + **/ careful here: the since won't match > xdgshell.h:95 > + > + /* > + * Which edge of the anchor should the popup be positioned around In case you wanted to make those comments doxygen comments: you need to use /** instead of /* Applies for all comments in this class. > xdgshell.h:176 > + > + void setup(zxdg_shell_v6 *xdgshellv6); > + lacking documentation including an @since. > xdgshell.h:222 > **/ > XdgShellPopup *createPopup(Surface *surface, Surface *parentSurface, > Seat *seat, quint32 serial, const QPoint &parentPos, QObject *parent = > nullptr); > If the comment is deprecated, the method should be deprecated as well. But I disagree on the point of deprecation. It is not deprecated if it's used with xdgShellv5. That should probably just be documented properly. > xdgshell.h:226 > + * Creates a new XdgShellPopup for the given @p surface on top of @p > parentSurface with the given @p positioner. > + **/ > + XdgShellPopup *createPopup(Surface *surface, XdgShellSurface > *parentSurface, const XdgPositioner &positioner, QObject *parent = nullptr); @since missing > xdgshell.h:229 > + > + /** > + * Creates a new XdgShellPopup for the given @p surface on top of @p > parentSurface with the given @p positioner. looks like wrong indentation. > xdgshell.h:231 > + * Creates a new XdgShellPopup for the given @p surface on top of @p > parentSurface with the given @p positioner. > + **/ > + XdgShellPopup *createPopup(Surface *surface, XdgShellPopup > *parentSurface, const XdgPositioner &positioner, QObject *parent = nullptr); @since missing > xdgshell.h:302 > + > + void setup(zxdg_surface_v6 *xdgsurfacev6, zxdg_toplevel_v6 *toplevel); > + missing documentation. > xdgshell.h:427 > > + /* > + * Set this surface to have a given maximum size same as with the other class: that's not a documentation comment. > xdgshell.h:500 > + * method. > + **/ > + void setup(zxdg_surface_v6 *xdgsurfacev6, zxdg_popup_v6 *xdgpopup6); @since missing > xdgshell.h:562 > + /** > + * > + **/ documentation missing > xdgshell_p.h:67 > + Q_UNUSED(parent) > + qWarning() << __func__ << " is not supported in this version"; > + return nullptr; Please no qWarning. Either qCWarning or not at all. > xdgshell_v6.cpp:28 > + > +#include <QDebug> > + I don't see any debug messages in this file REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D6047 To: davidedmundson, #plasma, graesslin Cc: graesslin, mart, plasma-devel, #frameworks, leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, hein, lukas