Code review comment for lp:~alan-griffiths/mir/even-NullWindowManager-configures-surface

> As the window manager has to implement some minimal (as opposed to null) behavior I think we
> should provide this (as opposed to documenting it).
>
> The use comes in two places:
>
> 1. the default window management logic is inconvenient for a number of our acceptance tests
> (e.g. they don't expect surfaces to be resized or placed). These use SkeletonWindowManager
> to disable the default.
>
> 2. Unity8 also finds the default window management logic is inconvenient and, as it currently
> uses different (older) customization points[*]. NullWindowManager provides a simple way to
> disable the default.

I am OK providing an (almost) null implementation for the use cases you mention. I would prefer it if it was not possible to inherit from this class, though.

To use the SkeletonWindowManager class as a base effectively, especially the methods that have a non-null implementation, the user needs to know what that class method do and do not do, which may change between releases, and I think this is a bigger burden for the user than just having to know that, e.g., set_surface_attribute() needs to configure the surface.

How about making SkeletonWindowManager "final", or perhaps a function "shared_ptr<WindowManager> make_skeleton_window_manager()" (and make the class internal)?