Code review comment for lp:~alan-griffiths/mir/a-simpler-server-setup

> 1. Should we reserve a buffer zone after accessors, overrides, and wrappers
> for future expansion? Otherwise we'll likely have interleaved
> accessors/overrides/wrappers areas.

A buffer zone where? These are non-virtual functions.

> 2. No tests?

True. If you can think of an effective way to test that the system gets configured & run correctly then I'd be delighted to add them.

In practice, this will get exercised properly when the acceptance tests get migrated to this interface. (Which is on my TODO list.)

> 3.
> s/hander/handler
> 111 + /// If set_command_line_hander is not called the default action
> is to exit by
> 114 + void set_command_line_hander(
> 115 + std::function<void(int argc, char const* const* argv)> const&
> command_line_hander);
> 365 + std::function<void(int argc, char const* const* argv)>
> command_line_hander{};
> 439 + std::function<void(int argc, char const* const* argv)> const&
> command_line_hander)
> 441 + if (command_line_hander)
> 442 + return std::make_shared<mo::DefaultConfiguration>(argc, argv,
> command_line_hander);
> 493 + auto const options = configuration_options(self->argc,
> self->argv, self->command_line_hander);
> 701 + mir::Server::set_command_line_hander*;

Fixed

> 4.
> s/add/Add
> s/an/a
> 125 + /// add an callback to be invoked when the server has been
> initialized,

Fixed

> 5.
> s/set/Set
> 107 + /// set a handler for any command line options Mir does not
> recognise.

Fixed.

> 6.
> 66 + * These are the commands used to start and stop.
> Based on this comment, to be consistent, should 'run' be named 'start'?
> 71 + /// Run the Mir server until it exits
> 72 + void run();