hein added inline comments.

INLINE COMMENTS

> notifications.cpp:828
> +{
> +    return QHash<int, QByteArray> {
> +        {IdRole, QByteArrayLiteral("notificationId")}, // id is QML-reserved

While this isn't an objection, I usually use QMetaEnum to compute this from the 
enum instead of having a block of stuff that needs to be synced.

> notifications.h:211
> +
> +    enum Roles {
> +        IdRole = Qt::UserRole + 1, ///< A notification identifier. This can 
> be uint notification ID or string application job source.

I recommend making this Q_ENUM and registering it; with libtm it turned out 
that it's useful to be able to call data() with a role from QML sometimes.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: hein, mart, nicolasfella, davidedmundson, ngraham, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

Reply via email to