← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~bac/lpsetup/mv-initialize-lxc into lp:lpsetup

 

Since commands.rst is reST, we can reST-style definition lists for the
"Terminology" and "install-lxc" sections (see
http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#definition-lists).

The line length in commands.rst seems inconsistent.  The longest line
looks to be 76 characters wide, which would suggest a 79 character max.
In that case there are several lines that seem to be prematurely
wrapped.  I suggest 79 character lines to match our source files.

The steps list in lpsetup/subcommands/inithost.py (line 381 of the diff)
should end with a comma after the last item in the list and the closing
parenthesis on its own line in order to be consistent with the other
subcommands.

The docstring for call_initialize_lxc can't decide if "lxc" should be
capitalized (and the subject and verb agreement is off, too).

You have a XXX comment on line 592 of the diff.

In running_in_container, if neither of the running-in-container or
lxc-is-container commands exist it will just return false.  It seems to
me that if that happens an exception should be raised instead.
-- 
https://code.launchpad.net/~bac/lpsetup/mv-initialize-lxc/+merge/114239
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/lpsetup/mv-initialize-lxc into lp:lpsetup.


References