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

Hey James!

Thanks for the review.

On Wed, Oct 17, 2012 at 5:42 PM, James Tunnicliffe
<email address hidden> 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.

Fixed them both, at least for the newly introduced functions.

>
> 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.

I do not like using \ either, removed and used a better formatting.
Thanks again!

--
Milo Casagrande
Infrastructure Engineer
Linaro.org <www.linaro.org> │ Open source software for ARM SoCs