[Differential] [Commented On] D3085: RFC: Use DBusMenu if available

[graesslin (Martin Gräßlin)]

graesslin added a comment.


  Overall significant less code than I expected - nice :-)

INLINE COMMENTS

> CMakeLists.txt:46
>  
> +find_package(Qt5PlatformSupport REQUIRED)
> +

version 5.7?

> FindQt5PlatformSupport.cmake:1
> +#.rst:
> +# FindQt5PlatformSupport

given that it has my copyright I assume you copied from kwin - that would be a reason to move it to ecm. We have two users.

> kdeplatformtheme.cpp:59
> +{
> +    static bool dbusGlobalMenuAvailable = checkDBusGlobalMenuAvailable();
> +    return dbusGlobalMenuAvailable;

can we really make that static? What about runtime changes?

> kdeplatformtheme.cpp:310
> +{
> +    if (isDBusGlobalMenuAvailable()) {
> +        return new QDBusMenuBar();

what happens if it becomes unavailable at runtime?

REPOSITORY
  rPLASMAINTEGRATION Integration for Qt applications in Plasma

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: broulik, #plasma
Cc: graesslin, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161017/9906fe63/attachment.html>