launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09798
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