Matt and Ethan:
Thanks for the re-review!
-Drew
On 6/24/11 12:04 PM, Ethan Quach wrote:
Drew,
Looks ok to me.
thanks,
-ethan
On 06/24/11 10:26, Drew Fisher wrote:
I've updated the webrev with Matt and Ethan's comments. I also fixed
test_auto_install_manifest.py to no longer require root have a bldenv
environment.
Could I get a quick eye on this, please?
http://cr.opensolaris.org/~drewfish/cr_7058653/
Thanks!
-Drew
On 6/24/11 1:52 AM, Matt Keenan wrote:
Drew,
Mostly looks good accept :
- Returning false in can_ignore_errors, is actually intentional.
The best fix here would actually be to completely remove this method
in it's entirety and the one instance where it is called.
- 2305 looks good.
cheers
Matt
On 06/24/11 03:11, Drew Fisher wrote:
Good evening!
Could I please get a code review for the following two bugs:
7058653 <http://monaco.us.oracle.com/detail.jsf?cr=7058653> undefined
variable disk_kd and discovered_disk in
checkpoints/target_selection.py
7058723 <http://monaco.us.oracle.com/detail.jsf?cr=7058723> Undefined
variable 'AIConfigruationError' in
auto_install/checkpoints/ai_configuration.py
http://cr.opensolaris.org/~drewfish/cr_7058653/
This is mostly a pep8/pylint cleanup run. No new code has been
introduced at this point other than what pylint complained about. I
ran
all of the unittests in the AI directory and there are no new test
suite
failures with these changes.
-----
Ethan: pylint still complains about something with
target_selection_zone.py:
[mox:checkpoints] > pylint target_selection_zone.py
************* Module target_selection_zone
W0221: 67:TargetSelectionZone.select_targets: Arguments number differs
from overridden method
I didn't want to change this one blindly without pinging you first to
make sure that I can make this change:
@@ -64,7 +64,7 @@
# set the platform
self.arch = platform.processor()
- def select_targets(self, from_manifest):
+ def select_targets(self, from_manifest, discovered):
'''Logic to select the targets for a zone.
so that it matches the method from the parent class.
Let me know if that's safe to do and I'll update the code before I
push
-----
Matt/Darren: I *think* I got target_selection.py right. I'm concerned
about line 2305, though. Can you confirm that I switched to the
correct
variable?
Thanks!
-Drew
_______________________________________________
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
