Drew,
Thanks for the review...
On 08/07/11 18:26, Drew Fisher wrote:
Ethan,
auto-install.py
---------------
220-223: commented code?
Removed.
787: unneeded parens around (self.manifest)
Removed.
1291: Is that a rogue '#' character?
Yes it is. Removed.
1299: I *think* you have an extra space before %(message)s ? Not
sure if that's intentional...
That's intentional. For output without a prefix, we want to align it
with the message portion with prefix, so there's an extra space to take
care of the ":" that's normally there with prefixed messages.
ips.py
------
Did all the transfer/ips unittests run ok?
Yeah, I ran all tests in lib/install_transfer/test/test_ips.py and
there are no errors.
89-111: I think it might make sense to make a generic logging
function instead of line after line of logging lines:
def ips_logger(self, message):
if self.show_stdout:
self.trans_logger.info(message)
else:
self.trans_logger.debug(message)
def eval_output_start(self):
self.ips_logger("Creating Plan ... Started.")
<...>
etc.
This function can be dropped in for dl_output, dl_output_done,
act_output, etc.
I've more or less made this change. It was applicable all the functions
in the class that output using the logger. See updated webrev.
195: Can you make this a simple 4 space indent rather than trying to
align parens?
done
183:
same exact comment for all of InstallFancyProgressTracker as 89-111
above
Not sure the same applies to these method because these methods don't
check self.show_stdout. Can you clarify?
439, 441: no need for the backslash
removed.
thanks,
-ethan
On 8/7/11 4:52 PM, Ethan Quach wrote:
Can I get a review for the following:
Webrev:
----------
https://cr.opensolaris.org/action/browse/caiman/equach/prog_output/webrev/
Bugs:
------
7052074 <http://monaco.us.oracle.com/detail.jsf?cr=7052074> zones
autoinstall needs more fine-grained progress reporting
7059740 <http://monaco.us.oracle.com/detail.jsf?cr=7059740> new AI
zone installation messages need improvement.
7057118 <http://monaco.us.oracle.com/detail.jsf?cr=7057118>
unwarranted "Version mismatch" warning
7068698 <http://monaco.us.oracle.com/detail.jsf?cr=7068698> AI
Install Phase progress not captured in the install_log
thanks,
-ethan
_______________________________________________
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
