Mir

Code review comment for lp:~alan-griffiths/mir/extend-demo-client-display-config

Revision history for this message
Chris Halse Rogers (raof) wrote :

Nit:
+ configuration_mode_up,
+ configuration_mode_down

We use C99; it'd be nice if you added a trailing ‘,’ here so that future additions don't also include a superfluous deletion.

Also nit:
- print_current_configuration(ctx->connection);
+ case XKB_KEY_p:print_current_configuration();

Missing newline between case XKB_KEY_p: and print_current_configuration()?

I find the use of the conf global strange, but actual use looks fine. The current code does no error-checking, so we're not losing anything.

Actually... this will crash if you press ‘p’ to print the display configuration before you do anything.

Maybe this should be factored into a little accessor function? Either that or print_current_configuration() needs a null-guard.

review: Needs Fixing

« Back to merge proposal