Code review comment for lp:~raof/mir/xserver-spawner

On Wed, 2014-01-29 at 03:09 +0000, Daniel van Vugt wrote:
> Review: Needs Fixing
>
> (1) "er" issues as described above.

Would you prefer XServerFactory?

> (2) I'm concerned about mentioning "X"-anything in the Mir source too. It feels like we're preventing Mir from being a success in its own right if it even has to mention X in its source. Even if the coupling is weak, we should strive for a better answer. Generalize, move it out-of-project, etc. We almost certainly don't want an "X" namespace. That's a reasonable indication that we're going in the wrong direction.

We're going to have to support running X11 apps for the foreseeable
future. Certainly for the life of 14.04, probably for the life of 16.04.

I don't think it's unreasonable for the Mir codebase to reflect the
importance of legacy X11 support.

Indeed, the actual implementations should be in a separate opt-in
libmirserverX11integration.so library, and will be once they start to
depend on X11 libraries.

>
> (3) You've introduced a build-dep on libx11-dev:
> +#include <X11/Xlib.h>
> Definitely don't do that, at least :)
>

Ah, yeah. This is currently only for the (disabled) acceptance test.
Once there's mirserver code that depends on X11 libs it'll be optional
and in a separate library.