Code review comment for lp:~nuclearbob/utah/docstring-cleanup

Revision history for this message
Max Brustkern (nuclearbob) wrote :

> On 04/08/2013 11:03 AM, Max Brustkern wrote:
> > That TODO was for me, and I forget to get to it before I finished the
> > merge. Whoops. I did want to ask you about that, when we're using a
> > file, do we want port to be 0 or would it be better as None? Does
> > the code to send the rsyslog arguments to the kernel command line
> > still check if that value is None? I don't think we need those
> > parameters if we're using a file, so I wanted to check if we were
> > still setting them.
>
> I think it can be None or 0. If you look at provisioning.py we have a check:
>
> if self.rsyslog.port:
> ... (setup rsylog'ing)
> else:
> ... (set up serial console logging)
>
> Maybe a smarter thing would be to make this more explicit like:
>
> if self.rsyslog.local_file()
>
> not sure.

I think I originally wrote things as 'if config.rsyslogport is not None' and I forgot that you had improved on that by testing the value directly, which will return False on a 0. Good stuff. I implemented the three things you said other than the MissingData, which I'll comment on separately.

« Back to merge proposal