Code review comment for lp:~rhansen/lightdm/multiseat-logging

Revision history for this message
Richard Hansen (rhansen) wrote :

> There seems to be no default value set for seats, i.e. if you do:
> $ cd tests
> $ DEBUG=1 ./test-autologin
>
> you get logs like:
>
> [+0.00s] DEBUG: Seat (null): Creating user session
>
> If the ID is NULL, id expect to see:
>
> [+0.00s] DEBUG: Seat: Creating user session
>
> or at least a valid ID.

OK, I'll see what I can do to improve it.

>
> You don't need to break at 80 characters

I really dislike lines longer than 80 characters, which is why I
usually wrap long lines when I change them. I'll unwrap, but I'll
feel like I'm violating my own principles. :)

See [1] for why don't I like lines longer than 80 characters.

[1] http://stackoverflow.com/a/111009/712605

>
> Also, now the logging is better, we have some redundant text.
>
> [+0.00s] DEBUG: Seat (null): Starting seat
>
> should become
>
> [+0.00s] DEBUG: Seat (null): Starting
>
> Though I wont block this merge on this. We can fix this in a later
> MP if you like.

I caught some of those (e.g., "starting session with PID=xxx"), but it
looks like I missed some. I'll do an audit.

« Back to merge proposal