Hi Matt,
Thanks for this fix; I have just one suggestion below.
On Sat, 2011-04-02 at 05:21 +0000, Matt Waddel wrote: [...] > === modified file 'linaro_image_tools/media_create/boards.py' > --- linaro_image_tools/media_create/boards.py 2011-04-01 14:18:41 +0000 > +++ linaro_image_tools/media_create/boards.py 2011-04-02 05:21:29 +0000 > @@ -287,7 +287,10 @@ > cmd_runner.run( > ['cp', '-v', uboot_bin, boot_disk], as_root=True).wait() > > - boot_script_path = os.path.join(boot_disk, cls.boot_script) > + if cls.boot_script: > + boot_script_path = os.path.join(boot_disk, cls.boot_script) > + else: > + boot_script_path = ''
I think it'd make more sense to just move the line you removed to the _make_boot_files() methods that use it. For instance, in OmapConfig we'd have:
boot_script_path = os.path.join(boot_dir, cls.boot_script) [...] make_boot_script(boot_env, boot_script_path) [...]
That means we'd duplicate the boot_script_path initialization in a few different methods, but it's just one line and I think that's cleaner than having it initialized in populate_boot() and passed all the way down to _make_boot_files()
« Back to merge proposal
Hi Matt,
Thanks for this fix; I have just one suggestion below.
On Sat, 2011-04-02 at 05:21 +0000, Matt Waddel wrote: image_tools/ media_create/ boards. py' image_tools/ media_create/ boards. py 2011-04-01 14:18:41 +0000 image_tools/ media_create/ boards. py 2011-04-02 05:21:29 +0000 True).wait( ) join(boot_ disk, cls.boot_script) join(boot_ disk, cls.boot_script)
[...]
> === modified file 'linaro_
> --- linaro_
> +++ linaro_
> @@ -287,7 +287,10 @@
> cmd_runner.run(
> ['cp', '-v', uboot_bin, boot_disk], as_root=
>
> - boot_script_path = os.path.
> + if cls.boot_script:
> + boot_script_path = os.path.
> + else:
> + boot_script_path = ''
I think it'd make more sense to just move the line you removed to the
_make_boot_files() methods that use it. For instance, in OmapConfig
we'd have:
boot_script_path = os.path. join(boot_ dir, cls.boot_script) boot_script( boot_env, boot_script_path)
[...]
make_
[...]
That means we'd duplicate the boot_script_path initialization in a few
different methods, but it's just one line and I think that's cleaner
than having it initialized in populate_boot() and passed all the way
down to _make_boot_files()