LGTM, Karen!

-Drew

On 7/14/11 3:57 PM, Karen Tung wrote:
Hi Drew,

Thanks for the review.  I have incorporated all your
suggestions in my code except 1 item.  Please see
my explanation inline for that item.

The updated webrev is at:

https://cr.opensolaris.org/action/browse/caiman/ktung/engine-bugs-updated/webrev/

Thanks,

--Karen

On 07/13/11 15:14, Drew Fisher wrote:
Karen,

usr/src/lib/install_engine/__init__.py
--------------------------------------

627:  use if not doc_list
630:  use if not self._checkpoints
869, 877, 884, 888:  use if cp_data is not None
1136:  maybe add "necessary" to the end of the message?

"First checkpoint is different than previous first checkpoint, no rollback
necessary"

1138:  remove the spaces inside the brackets:

resumable_cp_list[index-1]
 The spaces are added to make pep8 happy.  If
I don't have the spaces, I will get the following error from pep8.

ktung@rata {669} runpep8 *.py
__init__.py:1139:48: E225 missing whitespace around operator
                        resumable_cp_list[index-1])
                                               ^


1250-1259: 'file' is a reserved word in Python. Can we use something else here?

1257-1259:  why not use a one-liner here?
result = [f[1] for f in file_list_with_time]


Otherwise this looks great!  Thanks for fixing these!

-Drew




On 7/13/11 3:40 PM, Karen Tung wrote:
Hi,

I would like to get a review of my changes to fix the following 2 bugs:
7004000  <http://monaco.us.oracle.com/detail.jsf?cr=7004000>  Install engine 
enhancement to save original args/kwargs list before calling checkpoint's execute
7011388  <http://monaco.us.oracle.com/detail.jsf?cr=7011388>  resuming from 
failed checkpoints does not work
webrev:
https://cr.opensolaris.org/action/browse/caiman/ktung/engine_bugs/webrev/

Testing:

- I made sure that all the install engine and DC unit tests are run cleanly.
- I also manually tested various pause/resume DC execution scenarios and
made sure they all work as expected.

Thanks,

--Karen


_______________________________________________
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

Reply via email to