On Mon, 2014-12-01 at 09:12 +0100, Vratislav Podzimek wrote:
This also debug logs the result of the 'disks with potential stage1 targets' check.
I like this improvement with two comments:
- using the word 'volume' and accordingly named variables makes me
think these are (only) LVM or BTRFS volumes which is not true. 'partition' is also problematic, because LVM could, in some cases, be used, but it's more typical to use partitions for stage1 devices. Or what about 'device' or 'dev'? That fits best I'd say.
I thought 'volume' was more or less the standard term anaconda uses in this case - you know, when the first word that comes into your head is 'partition', but it's not actually quite correct =). So that's why I used it.
- see one neat pick below:
if platform.bootStage1ConstraintDict["format_types"]:
# sort the list of disks with those that contain possibly-valid
# stage1 partitions first, so we'll pick a disk with one as
# bootDrive if it has not been explicitly set: RHBZ #1168118
s1vols = [vol for vol in storage.devices if
getattr(vol.format, "type", None) in
platform.bootStage1ConstraintDict["format_types"]]
This, after having a better name, could be a generator instead of a list comprehension (using parentheses instead of square brackets).
What would be the advantage of that, in this case? The list isn't likely to be particularly long, or anything. I don't mind doing it, I'd just like to learn why ;)
In relation to this I'd like to mention that your Python skills became waaaay better over the recent months. Nice!
Thanks! It's not hard when the starting point is 'non-existent' ;)