Code review comment for lp:~milo/linaro-image-tools/method-refactor

Revision history for this message
James Tunnicliffe (dooferlad) wrote :

Thanks for tidying this up, but since it is a tidy-up branch, I am
going to be extra picky :-)

154 + def _old_format_extract_files(self):
155 + """
156 + Extract files for hwpack versions < 3.0.
157 + """

I think you have space to put the docstring on one line.
http://www.python.org/dev/peps/pep-0257/ likes docstrings that start
on the same line as the opening quote and is OK with the closing quote
on the same line.

There are several new functions without docstrings. Since this is a
re-factor, should probably include them.

161 + bootloader_package = \
162 + self.find_fetched_package(self.packages,
163 + self.config.bootloader_package)

I personally find the use of \ to continue a line ugly and think the
following form is easier on the eyes:

            bootloader_package = self.find_fetched_package(
                                        self.packages,
                                        self.config.bootloader_package)

Just personal preference though. There are several instances of this
type of line continuation that I think are only there because it was
how the old code was formatted.

Fix those two and it is fine (I assume you have done some test builds
and the unit tests pass).

« Back to merge proposal