Code review comment for lp:~clint-fewbar/ubuntu/natty/upstart/add-serial-console

On Wed, Jan 26, 2011 at 10:06:14AM -0000, James Hunt wrote:
> - Line 45: Might be simpler to write this as:
>
> [ ! -z `echo $arg | egrep "tty([0-9]+)"` ] && exit 0

Drive-by shell style nitpicking - I'd say this instead:

  echo "$arg" | egrep -q 'tty([0-9]+)' && exit 0

(i.e. use exit status rather than testing contents.)

> - I wonder if we should name the conf file "tty-serial.conf" rather than "ttyS.conf"
> to recognize that users might not be using serial device /dev/ttyS* but also /dev/ttyUSB*?

One thing to bear in mind with all of this is that debian-installer
already dynamically creates /etc/init/tty*.conf when you install on a
serial console, and the file name it uses comes straight from the device
name. Using /etc/init/ttyS.conf would certainly conflict with that in
an interesting way, which indeed suggests to me that something like
tty-serial.conf would be better, but we also ought to think about how
the whole thing plays together; maybe the installer code should be
obsoleted, but there's a fair bit of intelligence in there particularly
for some of the more obscure systems. In particular there are some
systems where the installer automatically sets up a serial console even
if you don't say console=.

Have a look at finish-install.d/90console in lp:finish-install.