On 10/22/10 05:03 PM, Karen Tung wrote:
Hi,
Thank you everyone for your feedback on the Install Engine code.
I have updated the files based on your comments. This is the updated
webrev against the latest slim_source gate:
http://cr.opensolaris.org/~ktung/engine_updated_1022/
This includes the files for target instantiation that I sent out
for a separate review yesterday. You can ignored them.
If you have any comments on the updated files, please send
them by COB next Tuesday 10/26, or sooner.
Thanks,
--Karen
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
Hi Karen,
Mostly just nits...
install_engine/Makefile
31 If you move the = so it's similar to 37, 39, then
you can line up __init__.py with checkpoint_data.py,
install_engine/__init__.py
o closing ''' for multiline docstring should be on separate line, You
have it that way on 113, would be good to have the other ones be consistent.
139 "Execution Cancelled" -> "Execution Canceled"
362 checkpoints -> checkpoint's
366,1104 no need for "+ \", implicit continuation.
373 "+" not needed
377, 382 PEP8 calls for the '+' to be on the line being continued
560-569 Should indicate what exceptions are raised ala 602
568 How sure are you that parameters will always be the same?
578 "session" -> " session"
905 Add "Raises" section
925-941 what should the behavior be if start_from and pause_before are
the same checkpoint? Or if pause_before is a checkpoint earlier than
start_from?
checkpoint.py
28 wants -> want
62 it's -> its
checkpoint_data.py
90 In error message:
Arguments %s is required -> Argument %s is required
ti.py
67-71 Should this code be removed now?
system-library-install.mf
o Should ti.py be in here?
test_engine.py
28 Comment is the same as that at top of test_cp_data.py. Would be nice
to have different comment that gives indication of different between
two files.
130 Comment says that tests do not write to disks, but looks like it's
making directory at 163
engine_test_utils.py
28 convinence -> convenience
28 other test suites that uses -> by other test suites that use
Sue
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
