For the most part this looks really good. target_selection.py is mostly
absurd (in a good way) though :)
usr/src/cmd/auto-install/auto_install.py
----------------------------------------------------
Why the switch from "/" to "." in register_checkpoint() calls? I
thought they had to be slashes?
156, 394, 438, 572: FIXME comment
239: combine with 240
if linecache.getline(manifest, 1)[:2]:
462: Let's talk offline about this. I'm sure we can move this code
block up higher
1086: remove unneeded line
usr/src/cmd/distro_const/checkpoints/pre_pkg_img_mod.py
--------------------------------------------------------------------------------
I'm surprised by this showing up here. This was my change for CR
7041226. Might want to check for other merge issues? Is this a merge
error?
usr/src/lib/install_common/__init__.py
---------------------------------------------------
118-121: Same issue as above. Keith fixed this as part of CR <can't
find this right now>
407-420: "file" is a reserved word in python. Can you rename?
usr/src/lib/install_target/Makefile && controller.py
------------------------------------------------------------------
Potentially another merge error? Wasn't this Dermot's push of
controller.py to slim_source?
usr/src/cmd/auto-install/checkpoints/target_selection.py
-----------------------------------------------------------------
503, 537: s/This/this
826-835: Why not just use zpool.get_first_child(name=zvol.name,
class_type=Zvol) ?
837-850: Why not just use discovered.get_descendants(class_type=BE)
instead of grabbing all the zpools?
1005 - 1018: You can simply this by using the default argument to
dict.get()
if zpool.exists:
retsize = Size(zpool.get("size", "0b"))
else:
retsize = Size("0b")
return retsize
1073, 2643, 2920: You're looping through a list of Logical DOC objects
but the DTD specifies that only 1 Logical element can exist:
<!ELEMENT target (disk*, logical?)>
1121, 1129, 2411: TODO comment
1142-1145: Why are these lines here?
1433: align indent with [ on 1432
1446: align
1502: s/iv/if
1509, 1889: you don't need the call to .keys()
-Drew
On 5/18/11 4:46 PM, Darren Kenny wrote:
Hi,
I think it's about time I got out another version of the code review for the CUD
AI project.
I've uploaded the webrev at:
http://cr.opensolaris.org/~dkenny/cud_ai-to-slim-2/
And for anyone that's reviewed the code before, there is a diff:
http://cr.opensolaris.org/~dkenny/cud_ai-to-slim-2-diffs/
target_selection.py has probably changed the most, so if time is short, we'd
really appreciate you reviewing that file at least.
If at all possible could you please provide any feedback by Friday COB.
Thanks,
Darren.
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
