| 2011-04-06 13:04:34 |
Alexander Sack |
description |
On Mon, 2011-04-04 at 09:25 +0000, Alexander Sack wrote:
> Hi salgado,
>
> is it ok to merge the current state? we still have some changes
> pending wrt what artifacts we will really release and so. Jeremy and
> me discussed this and we think refactoring/reuse of code effort should
> be done in one batch before the 11.05 release.
I agree the refactoring to avoid code duplication can be done later, and
I said so in my review. The most important change I proposed was to use
separate config classes for android as that would be much cleaner but
more importantly would make for a better baseline on where to build
further android-related changes.
I'd be happy to postpone this provided that there was an XXX in the code
for it to be fixed later, but this really was a trivial change and I see
in the last merge proposal that most of what I asked for is already
done, so I see no reason why this shouldn't be done now. All that is
left to do is create the AndroidBoardConfigMixin class as I described
and move get_android_sfdisk_cmd() there (as get_sfdisk_cmd()). With
that we can then get rid of the image_type argument and the ugly change
below.
- sfdisk_cmd = board_config.get_sfdisk_cmd(
- should_align_boot_part=should_align_boot_part)
+ if image_type == "ANDROID":
+ sfdisk_cmd = board_config.get_android_sfdisk_cmd(
+ should_align_boot_part=should_align_boot_part)
+ else:
+ sfdisk_cmd = board_config.get_sfdisk_cmd(
+ should_align_boot_part=should_align_boot_part) |
From: https://code.launchpad.net/~jeremychang/linaro-image-tools/android/+merge/56113
On Mon, 2011-04-04 at 09:25 +0000, Alexander Sack wrote:
> Hi salgado,
>
> is it ok to merge the current state? we still have some changes
> pending wrt what artifacts we will really release and so. Jeremy and
> me discussed this and we think refactoring/reuse of code effort should
> be done in one batch before the 11.05 release.
I agree the refactoring to avoid code duplication can be done later, and
I said so in my review. The most important change I proposed was to use
separate config classes for android as that would be much cleaner but
more importantly would make for a better baseline on where to build
further android-related changes.
I'd be happy to postpone this provided that there was an XXX in the code
for it to be fixed later, but this really was a trivial change and I see
in the last merge proposal that most of what I asked for is already
done, so I see no reason why this shouldn't be done now. All that is
left to do is create the AndroidBoardConfigMixin class as I described
and move get_android_sfdisk_cmd() there (as get_sfdisk_cmd()). With
that we can then get rid of the image_type argument and the ugly change
below.
- sfdisk_cmd = board_config.get_sfdisk_cmd(
- should_align_boot_part=should_align_boot_part)
+ if image_type == "ANDROID":
+ sfdisk_cmd = board_config.get_android_sfdisk_cmd(
+ should_align_boot_part=should_align_boot_part)
+ else:
+ sfdisk_cmd = board_config.get_sfdisk_cmd(
+ should_align_boot_part=should_align_boot_part) |
|
