[Merge] lp:~nick-dedekind/qtubuntu/menuTheme into lp:qtubuntu

Gerry Boland gerry.boland at canonical.com
Thu Aug 11 18:34:13 UTC 2016


Review: Needs Fixing

Hey,
this was bigger than I was expecting!

Questions: 
1. how do I test this?
2. is the theme plugin totally independent from the QPA plugin? I see you moved the QPA's theme code into the Integration code. Why? Just to indicate the majority of the theming is elsewhere?


~/dev/projects/qtubuntu/menuTheme/src ⮀ l
src.pro  ubuntuappmenu/  ubuntumirclient/
~/dev/projects/qtubuntu/menuTheme/src ⮀ cd ubuntu<TAB><TAB>
ubuntuappmenu/    ubuntumirclient/
/me hating how we prefix everything with "ubuntu".


I'm just looking at the QPA for now
- you add persistentSurfaceId - can it be a QByteArray instead of QString?

+    // queue the windowPropertyChanged signal. If it's emitted directly, the platformWindow will not yet be set for the window.
+    QMetaObject::invokeMethod(mNativeInterface, "windowPropertyChanged", Qt::QueuedConnection,
+                              Q_ARG(QPlatformWindow*, this),
+ Q_ARG(QString, "persistentSurfaceId"));
How does a persistent Surface ID ever change?

+ return QStringList("ubuntuappmenu");
QStringLiteral please.

-Q_LOGGING_CATEGORY(ubuntumirclientInput, "ubuntumirclient.input", QtWarningMsg)
+Q_LOGGING_CATEGORY(ubuntumirclientInput, "ubuntu.mirclient.input", QtWarningMsg)
Why? This breaks the standard elsewhere in the code. Check the README too.
-- 
https://code.launchpad.net/~nick-dedekind/qtubuntu/menuTheme/+merge/296997
Your team Ubuntu Phablet Team is subscribed to branch lp:qtubuntu.



More information about the Ubuntu-reviews mailing list