Code review comment for lp:~muktupavels/lightdm/improve-multiseat-support

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

> This fails when you run 'make check'. Specifically the xdg-seat test (cd tests
> ; ./test-xdg-seat). The fix is quite trivial but you should make it to show
> that you know the tests and think if this is the correct behaviour.

Is it good now? Now all tests completes successfully. From xdg-seat.conf removed VT check. Only main seat - seat0 should know about VT (from docomentation - http://www.freedesktop.org/wiki/Software/systemd/writing-display-managers/). So if seat is not seat0 than vt should not be set.

> The removal of the VT check in seat_xlocal_get_active_session () doesn't
> really make sense. In a multi-seat case you should probably check
> seat_get_string_property (seat, "xdg-seat") instead and do something different
> than checking if any session is on this VT.

Added back VT check.

> You should replace:
> (xdg_seat && strcmp (xdg_seat, "seat0") == 0)
> with
> g_strcmp0 (xdg_seat, "seat0") == 0
> as this is more readable.

Replaced.

« Back to merge proposal