Merge lp://staging/~alan-griffiths/qtmir/port-to-msh-Shell into lp://staging/qtmir

FAILED: Continuous integration, rev:315
http://jenkins.qa.ubuntu.com/job/qtmir-ci/206/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/57/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/57/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/206/rebuild

Needs libmirserver-dev rev bump in debian/control.

Looks good otherwise.

LGTM

FAILED: Continuous integration, rev:316
http://jenkins.qa.ubuntu.com/job/qtmir-ci/208/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/59/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/59/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/208/rebuild

You're gonna hate me, but Qt coding style would impose:
std::shared_ptr<mir::shell::InputTargeter> const& input_targeter
instead of
const std::shared_ptr<mir::shell::InputTargeter> &inputTargeter
i.e.
1. const before type
2. & or * attached to variable name
3. variables using camelCase, not underscore. Only exception are class members which are prefixed with "m_"

Please attach the checklist too: https://wiki.ubuntu.com/Process/Merges/Checklists/QtMir
Is good otherwise

> You're gonna hate me, but Qt coding style would impose:
> std::shared_ptr<mir::shell::InputTargeter> const& input_targeter
> instead of
> const std::shared_ptr<mir::shell::InputTargeter> &inputTargeter

I think you've got that the right way around. ;)

But I've changed it.

> i.e.
> 1. const before type
> 2. & or * attached to variable name
> 3. variables using camelCase, not underscore. Only exception are class members
> which are prefixed with "m_"

I note that qtmir is far from consistent in this. E.g.

251 - MirPlacementStrategy(std::shared_ptr<mir::shell::DisplayLayout> const& display_layout);

> Please attach the checklist too:
> https://wiki.ubuntu.com/Process/Merges/Checklists/QtMir

* Did you perform an exploratory manual test run of the code change and any related functionality?

No

 * Did CI run pass? If not, please explain why.

No, changes depend on newer Mir version

> Is good otherwise

> > You're gonna hate me, but Qt coding style would impose:
> > std::shared_ptr<mir::shell::InputTargeter> const& input_targeter
> > instead of
> > const std::shared_ptr<mir::shell::InputTargeter> &inputTargeter
>
> I think you've got that the right way around. ;)

D'oh. Thanks!

> But I've changed it.
>
> > i.e.
> > 1. const before type
> > 2. & or * attached to variable name
> > 3. variables using camelCase, not underscore. Only exception are class
> members
> > which are prefixed with "m_"
>
> I note that qtmir is far from consistent in this. E.g.
>
> 251 - MirPlacementStrategy(std::shared_ptr<mir::shell::DisplayLayout>
> const& display_layout);

Indeed, I had a mad notion to use mir style code for mir things, and Qt for Qt things. However now I'm slowly just moving to the Qt style.
Thanks for doing it!

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yep, tested fine
 * Did CI run pass? If not, please explain why.
Should do, just waiting for it before Top Approve

"""
587 -SurfaceConfigurator::SurfaceConfigurator()
588 -{
589 - qRegisterMetaType<MirSurfaceAttrib>("MirSurfaceAttrib");
590 -}
"""

We need to register MirSurfaceAttrib as a metatype otherwise signal-slot connections that have a parameter or this type will fail.

Please apply this patch to your branch:

""""
=== modified file 'src/modules/Unity/Application/plugin.cpp'
--- src/modules/Unity/Application/plugin.cpp 2014-10-06 11:37:56 +0000
+++ src/modules/Unity/Application/plugin.cpp 2015-02-05 09:30:09 +0000
@@ -78,6 +78,7 @@ class UnityApplicationPlugin : public QQ
         qRegisterMetaType<qtmir::Session*>("Session*");
         qRegisterMetaType<qtmir::SessionInterface*>("SessionInterface*");
         qRegisterMetaType<qtmir::SessionModel*>("SessionModel*");
+ qRegisterMetaType<MirSurfaceAttrib>("MirSurfaceAttrib");

         qmlRegisterUncreatableType<unity::shell::application::ApplicationManagerInterface>(
                     uri, 0, 1, "ApplicationManagerInterface", "Abstract interface. Cannot be created in QML");
""""

The patch in a more readable way:
http://paste.ubuntu.com/10069953/

It's ok now.

+ MirShell *const m_shell;
"const MirShell *m_shell" please

> + MirShell *const m_shell;

That's a constant pointer to a (non-constant) MirShell

> "const MirShell *m_shell" please

That's a (non-constant) pointer to a constant MirShell

Do you really want "const (MirShell *)m_shell?

You're totally right, objection withdrawn