broulik added inline comments.

INLINE COMMENTS

> notifybysnore.cpp:159
> +    } else {
> +        Qstring iconPath = QString(m_iconDir.path() + QLatin1Char('/')
> +                            + QString::number(notification->id()) + 
> QStringLiteral(".png"));

Did you even compile this?
Also, this is the same in both branches, move it above the `if` and also make 
it `const`

> notifybysnore.cpp:161
> +                            + QString::number(notification->id()) + 
> QStringLiteral(".png"));
> +        QIcon app_icon = qApp->windowIcon();
> +    // We limit the icon size to 1024x1024 as it is the highest supported by 
> Windows

Check for `isNull()`

> notifybysnore.cpp:163
> +    // We limit the icon size to 1024x1024 as it is the highest supported by 
> Windows
> +        QPixmap pixmap = app_icon.pixmap(app_icon.actualSize(QSize(1024, 
> 1024)));
> +        pixmap.save(iconPath, "PNG");

The explicit call to `actualSize` shouldn't be neccessary

> notifybysnore.cpp:165
> +        pixmap.save(iconPath, "PNG");
> +        arguments   << QStringLiteral("-p") << iconPath;
>      }

Whitespace

REPOSITORY
  R289 KNotifications

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

To: brute4s99, #frameworks
Cc: broulik, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

Reply via email to