graesslin requested changes to this revision. graesslin added a comment. This revision now requires changes to proceed.
Overall lots of documentation is missing. From API layout that looks much better now, but we still have a few places where "Unstable" is needlessly in the API. Most important in Display the create method is not forward compatible by not having an enum to describe which interface version should be created. INLINE COMMENTS > registry.h:527-528 > + > + zxdg_exporter_v1 *bindXdgExporterUnstableV1(uint32_t name, uint32_t > version) const; > + zxdg_importer_v1 *bindXdgImporterUnstableV1(uint32_t name, uint32_t > version) const; > ///@} please add documentation > registry.h:940-941 > + > + XdgExporter *createXdgExporterUnstable(quint32 name, quint32 version, > QObject *parent = nullptr); > + XdgImporter *createXdgImporterUnstable(quint32 name, quint32 version, > QObject *parent = nullptr); > ///@} please add documentation > registry.h:940-941 > + > + XdgExporter *createXdgExporterUnstable(quint32 name, quint32 version, > QObject *parent = nullptr); > + XdgImporter *createXdgImporterUnstable(quint32 name, quint32 version, > QObject *parent = nullptr); > ///@} Why did you keep the suffix "Unstable"? > registry.h:1132-1133 > + > + void exporterUnstableV1Announced(quint32 name, quint32 version); > + void importerUnstableV1Announced(quint32 name, quint32 version); > ///@} please add documentation > registry.h:1288-1290 > + void exporterUnstableV1Removed(quint32 name); > + void importerUnstableV1Removed(quint32 name); > ///@} please add documentation > display.h:223 > > + XdgForeignInterface *createXdgForeignUnstableInterface(QObject *parent = > nullptr); > /** please add documentation. Also the common way would be to have an enum to describe which interface should be created, but therefore drop the Unstable from the API name. > xdgforeign_interface.h:37 > +class XdgImporterUnstableV1Interface; > + > +class KWAYLANDSERVER_EXPORT XdgForeignInterface : public QObject Something like an enum ForeignInterfaceVersion is missing, please compare TextInputInterface > xdgforeign_v1_interface.cpp:260 > + > + QPointer<XdgImportedUnstableV1Interface> imp = new > XdgImportedUnstableV1Interface(s->q, surface); > + imp->create(s->display->getConnection(client), > wl_resource_get_version(resource), id); what's an imp? Could you please write full variable names? REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7369 To: mart, #plasma, #kwin, davidedmundson, graesslin Cc: davidedmundson, graesslin, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
