sredman added inline comments. INLINE COMMENTS
> CMakeLists.txt:43 > +if (WIN32) > + if (MSVC) > + find_package(LibSnoreToast REQUIRED) Do you know whether this requires MSVC, or can SnoreToast.exe be built with MSVC and KNotifications be built with an unspecified compiler? > notifybysnore.cpp:44 > + > +// !DOCUMENT THIS! apps must have shortcut appID same as > app->applicationName() > +NotifyBySnore::NotifyBySnore(QObject* parent) : Did this end up being documented? > notifybysnore.cpp:73 > + case SnoreToastActions::Actions::Clicked :{ > + qDebug() << " User clicked on the > toast."; > + break; Add a category to qDebug, like `qCDebug(LOG_KNOTIFICATIONS) << "Message";`. I can't figure out where `LOG_KNOTIFICATIONS` is coming from, but other backends seem to use it :) > notifybysnore.cpp:117 > + if (m_notifications.find(notification->id()) != m_notifications.end() || > notification->id() == -1) { > + qDebug() << "AHAA ! Duplicate for ID: " << notification->id() << > " caught!"; > + return; Maybe this log line should be different? :) > notifybysnore.cpp:155 > + << QStringLiteral("-appID") << app->applicationName(); > +; > + proc->start(program, arguments); Does this `;` belong to something? > notifybysnore.h:48 > + QString program = QStringLiteral("SnoreToast.exe"); > + // QProcess *proc; > + QLocalServer *server; Delete commented code REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D21661 To: brute4s99, broulik, sredman, vonreth, albertvaka Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns