D21661: add snoretoast backend for KNotifications on Windows

Simon Redman noreply at phabricator.kde.org
Sat Jun 8 03:21:23 BST 2019


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190608/bb70170d/attachment.html>


More information about the Kde-frameworks-devel mailing list