Hi Drew,
Some website funkiness blocked me from logging in and posting this follow up
review of physical.py so I mailed it to you privately as you know. I'm just
re-posting my review comments here to keep things together.
Cheers,
Niall
physical.py
-----------
130:
try:
partition.size = Size(size.get("val"))
partition.start_sector = int(size.get("start_sector", 0))
except:
# catch any failure
raise ParsingError("Partition size element has invalid " + \
"'start_sector' attribute")
It seems that either of the two expressions could raise a ParsingError, yet
the error message assumes it's always the second one (start_sector).
281:
def remaining_space(self):
""" remaining_space() - class property to return a Size object of the
remaining overall space available on the Partition
"""
Nit: Is this not an object (instance) property rather than a class properry?
299: Same as 281
308: Same as 281
317: Same as 281
1081: Same as 281
299: def is_logical(self):
Suggestion - not something I'll get hung up on :-)
How about making this just return the logical inverse of is_primary?
eg. if self.is_primary:
return False
308: def is_extended():
Similar to above, how about:
if self.is_primary() and \
self.part_type in Partition.EXTENDED_ID_LIST:
667:
if 5 <= index <= 36:
bootid = Partition.INACTIVE
Why are you not comparing against the const "const.FD_NUMPART" as used in
Partition.is_primary() eg.
if const.FD_NUMPART < index <= 36
Where does 36 come from? Might be useful to add a short comment.
901: # XXX does this command really work?
Does it? :)
942, 947:
Nit: number_parts = number_parts + 1 -> number_parts += 1
--
This message posted from opensolaris.org
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
