D29420: Generate DBus interface

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> notifybypopup.cpp:115
>  
> -     bool connected = QDBusConnection::sessionBus().connect(QString(), // from any service
> -                                                               QString::fromLatin1(dbusPath),

Previously it would accept the signal from any service which I find odd, though. Could you maybe check git logs to see if there was a reason for this?
It should survive restarts anyway and the service name is defined, so I really don't see why it used to be a blind connect.

> notifybypopup.cpp:364
> +        QDBusPendingReply<uint> reply = *watcher;
> +        notifications.insert(reply.argumentAt<0>(), notification);
> +    });

You have `watcher` (which is parented to `notification`) and `notification` as contexts, but in the lambda you also access `this`. This looks dangerous. How about using `this` instead of `notification` as context object?

> notifybypopup.cpp:376
> +
> +    QDBusPendingCall call = dbusInterface.GetCapabilities();
> +

This is `QDBusPendingReply<QStringList>` (or just `auto`...)

> notifybypopup.cpp:385
> +        popupServerCapabilities = capabilities;
> +        dbusServiceCapCacheDirty = false;
> +

Unrelated: I was wondering, do we actually monitor the notification service changing and make them dirty again?

> notifybypopup.cpp:388
> +        // re-run notify() on all enqueued notifications
> +        for (int i = 0, total = notificationQueue.size(); i < total; ++i) {
> +            q->notify(notificationQueue.at(i).first, notificationQueue.at(i).second);

range for?

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D29420

To: nicolasfella, #frameworks, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200505/4be4452b/attachment-0001.html>