Code review comment for lp:~unity-team/qtmir/qtmir.api

+++ src/platforms/mirserver/displayconfigurationstorage.cpp

last line
+}
missing a "// namespace qtmir"

+++ src/platforms/mirserver/guiserverapplication.cpp
+QSharedPointer<QMirServer> mirServer;
does it need to be shared?

around line 36, please add "// namespace" after the anonymous closing brace. And at EOF, another comment please. I get lost with namespaces.

+ : QGuiApplication((init(argc, argv, options), argc), argv) // comma operator to ensure init called before QGuiApplication
clever!

+++ src/platforms/mirserver/miral/persist_display_config.cpp
+ if (mode.size == newMode.size && mode.vrefresh_hz == newMode.refresh_rate) {
refresh_rate comparison is a floating point comparison, could be unreliable. Can you use something like qFuzzyCompare?

+++ src/platforms/mirserver/mirserverintegration.h
- QScopedPointer<QMirServer> m_mirServer;
+ QSharedPointer<QMirServer> m_mirServer;
Does it really need to be shared? AFAICS nobody else makes a copy of the QSharedPointer after construction.

+++ src/platforms/mirserver/qmirserver.h
+ static QSharedPointer<QMirServer> create(int &argc,
+ char **argv);
It's your big entry point header file, make it pretty! Do line up these arguments, or just have on single line.

+++ src/platforms/mirserver/qmirserver_p.cpp
+auto buildDisplayConfigurationPolicy()
+-> std::shared_ptr<miral::DisplayConfigurationPolicy>
I'm not a fan of this method definition style. Please stick to the old fashioned one.