Code review comment for lp://staging/~mabac/linaro-image-tools/snowball-android-startfiles-hack

Revision history for this message
Mattias Backman (mabac) wrote :

On Mon, Feb 20, 2012 at 7:51 PM, Guilherme Salgado
<email address hidden> wrote:
> Wow, nice, lots of new tests!  This looks quite good; the only comment I have is about snowball_config(chroot_dir), which has no docstring and return a two-tuple where one value is a hard-coded boolean so there's no way to know what it means. I'd prefer if the hard-coded value were in a class attribute and snowball_config(chroot_dir) returned just the path to the config dir, but having just a docstring on snowball_config() would be ok as well.

I did the classproperty fix before merging.

Also mpoirier re-acked the change via email.

> --
> https://code.launchpad.net/~mabac/linaro-image-tools/snowball-android-startfiles-hack/+merge/93430
> You are the owner of lp:~mabac/linaro-image-tools/snowball-android-startfiles-hack.

« Back to merge proposal