Merge lp://qastaging/~zyga/linaro-image-tools/hacking into lp://qastaging/linaro-image-tools/11.11
Proposed by
Zygmunt Krynicki
Status: | Rejected |
---|---|
Rejected by: | Loïc Minier |
Proposed branch: | lp://qastaging/~zyga/linaro-image-tools/hacking |
Merge into: | lp://qastaging/linaro-image-tools/11.11 |
Diff against target: |
77 lines (+15/-8) 2 files modified
linaro_media_create/check_device.py (+9/-6) linaro_media_create/partitions.py (+6/-2) |
To merge this branch: | bzr merge lp://qastaging/~zyga/linaro-image-tools/hacking |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Guilherme Salgado (community) | Needs Fixing | ||
Review via email: mp+49615@code.qastaging.launchpad.net |
Description of the change
Display the selected device more clearly
To post a comment you must log in.
Unmerged revisions
- 300. By Zygmunt Krynicki
-
Add a hack that makes it possible to run batch jobs
- 299. By Zygmunt Krynicki
-
Add sleep after partitioning the medium
- 298. By Zygmunt Krynicki
-
Increase the delay after sync to 3 seconds.
This fixes building images on my system due to race condition between udisks and sfdisk
- 297. By Zygmunt Krynicki
-
Hilight selected device so that it's easier to read the partition/device list
Hi Zygmunt,
Thanks for your change, but I think what we really want here is to list
the selected device and all its partitions at the top of the list or
maybe even on a separate table, with proper headings instead of just "I
see...".
However, I think your changes can be an improvement, and it should be OK
to land just them if you don't have the time to do (or don't feel like
doing) the other changes. I have just a few suggestions.
First, the length of the new column is a bit too small so it feels like
instead of having two columns (Selected, and Device), we have just one
(Selected Device).
review needsfixing
On Mon, 2011-02-14 at 12:10 +0000, Zygmunt Krynicki wrote: media_create/ check_device. py' media_create/ check_device. py 2011-01-28 19:50:48 +0000 media_create/ check_device. py 2011-02-14 12:09:46 +0000 devices( hilight_ device= None): bus_and_ udisks_ iface() get_dbus_ method( 'EnumerateDevic es')() object( "org.freedeskto p.UDisks" , path) property( 'DeviceIsPartit ion', device, path): property( 'partition- size', device, path)
[...]
> === modified file 'linaro_
> --- linaro_
> +++ linaro_
> @@ -65,10 +65,10 @@
> return True
>
>
> -def _print_devices():
> +def _print_
> """Print disk devices found on the system."""
> bus, udisks = _get_system_
> - print '%-16s %-16s %s' % ('Device', 'Mount point', 'Size')
> + print '%-8s %-16s %-16s %s' % ('Selected', 'Device', 'Mount point', 'Size')
> devices = udisks.
> for path in devices:
> device = bus.get_
> @@ -81,11 +81,13 @@
>
> if _get_dbus_
> part_size = _get_dbus_
> - print '%-16s %-16s %dMB' % (
> + print '%-8s %-16s %-16s %dMB' % (
> + "------->" if hilight_device == device_file else "",
This is never going to print the marker as we're dealing with partitions tion').
here ('DeviceIsParti
I'd also prefer if you used regular if/else blocks instead of the short property( )" block so that we avoid duplicating it in the else
form here. Maybe you can even move it before the "if
_get_dbus_
block below?
> device_file, mount_point, part_size / 1024**2) property( 'device- size', device, path) exist(device) : devices( hilight_ device= device) device( device) : device_ partitions_ not_mounted( device)
> else:
> device_size = _get_dbus_
> - print '%-16s %-16s %dMB' % (
> + print '%-8s %-16s %-16s %dMB' % (
> + "------->" if hilight_device == device_file else "",
> device_file, mount_point, device_size / 1024**2)
>
>
> @@ -121,7 +123,7 @@
> """
> if _does_device_
> print '\nI see...'
> - _print_devices()
> + _print_
> if _select_
> _ensure_
> return True
>
-- /launchpad. net/~salgado>
Guilherme Salgado <https:/