sebas added a comment.
a few questions and suggestions, feedback welcome... INLINE COMMENTS > shell_client_test.cpp:129 > +{ > +#define CLEANUP(name) \ > + if (name) { \ aren't you going overboard here? I don't think this makes the backtrace a lot easier to read... > shell_client_test.cpp:180 > + surface->attachBuffer(Buffer::Ptr()); > + surface->commit(Surface::CommitFlag::None); > + QVERIFY(hiddenSpy.wait()); I wonder if this should be the default arg in the API. What do you think? > shell_client_test.cpp:190 > + QCOMPARE(clientAddedSpy.count(), 1); > + QVERIFY(windowShownSpy.wait()); > + QCOMPARE(clientAddedSpy.count(), 1); Also check windowShownSpy.count()? > shell_client_test.cpp:202 > + surface.reset(); > + QVERIFY(windowClosedSpy.wait()); > +} Check count here? > wayland_server.cpp:251 > + ShellClient *c = dynamic_cast<ShellClient*>(t); > + if (!c) { > + return; worth warning here, or is this a valid case? (Looks like an application bug if this return is hit) REPOSITORY rKWIN KWin REVISION DETAIL https://phabricator.kde.org/D1784 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: graesslin, #kwin, #plasma_on_wayland Cc: sebas, plasma-devel
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel