Code review comment for lp:~yellow/launchpad/lxcsetup

Here's what I have this far:

lxc-create and lxc-start use the "-n" option to specify the instance
name, we might want to do the same.

It would be nice if the script used the current user info (login name)
if none are provided as arguments. Taking that a step further, bzr
whoami produces consistent, easily parsable results that could make
--email and --name optional as well.

...Later: oh! The user info is required because the script is expecting
to be run as root. I suppose the easiest thing would be to leave it
as-is, but it'd be really nice to not have to provide all of those
arguments. Perhaps the script can be re factored into two halves: the
first starts as the user in question, and the second part is run under
sudo (prompting the user for a password if nectary).

I'm a big fan of doctest, but it's LP policy to not introduce any new
(non-documentation) doctests, so the embedded doctests should be
translated into unittest-style tests.

Is it really necessary to skip the "sudo" (in launchpad-database-setup)
if already running as root? I would expect the sudo to be a noop in
that scenario. Similarly, "sudo -u postgres" should work equally well
for root as it does for non-root users.

We've mostly transitioned away from the %-style string construction and
switched to the new .format() string method so the script should avoid
constructing strings with %.

On line 210 of the diff: The summary line of a docstring should fit on a
single line.

The file_append function doesn't have to rewrite the entire file.
Instead the file can be opened in append mode and the new line appended.

Also, when appending a line to a file you should check to see if the
last line of the file doesn't have a trailing newline, otherwise you can
end up adding your string to the last line of the file instead of
appending an entirely new line.

Since this script requires Python 2.7, I would use the print function
instead of the print keyword (from __future__ import print_function).

On line 366 of the diff, the description of the --name argument leaves
off the "r" in "for" ("used fo bzr").

The parenthesis around the multi-line constructions of the "help"
arguments aren't necessary.

Requiring an SSH private key without passphrase gives me some security
shivers and may raise a few eyebrows.