D5538: Implement QPlatformTheme::fileIconPixmap()

[Martin Flöser]

graesslin requested changes to this revision.
graesslin added a comment.
This revision now requires changes to proceed.


  Please fix the style issues.

INLINE COMMENTS

> kdeplatformtheme.cpp:124-125
> +#if QT_VERSION < QT_VERSION_CHECK(5, 8, 0)
> +QPixmap KdePlatformTheme::fileIconPixmap(const QFileInfo &fileInfo, const QSizeF &size,
> +                                   QPlatformTheme::IconOptions iconOptions) const
> +{

please either use indentation or use a single line

> kdeplatformtheme.h:48
> +    QIcon fileIcon(const QFileInfo &fileInfo,
> +                           QPlatformTheme::IconOptions iconOptions) const Q_DECL_OVERRIDE;
> +#else

please use override instead of Q_DECL_OVERRIDE

> kdeplatformtheme.h:71
>      QScopedPointer<X11Integration> m_x11Integration;
> -
>  };

nitpick: unrelated removal of empty line

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: eshalygin, #plasma, markg, graesslin
Cc: graesslin, ltoscano, broulik, markg, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170507/ef1e8392/attachment.html>