Thanks Jack for the review.
I would need 1 more reviewer for this.
Thanks,
Nirmal
On 07/19/12 00:29, Jack Schwartz wrote:
Thanks, Nirmal. LGTM.
Jack
On 07/18/12 11:23 AM, Nirmal Agarwal wrote:
Hi Jack,
Thanks for the review. I have modified the fix as per your comments.
Please find the webrev :
https://cr.opensolaris.org/action/browse/caiman/nirmal27/7179899/webrev/
I re-ran the tests to confirm that manifest file gets deleted.
Thanks,
Nirmal
On 7/18/2012 10:46 PM, Jack Schwartz wrote:
Hi Nirmal.
Here are my comments:
test_mw_dry_run.py:
61: I suggest using os.path.exists() instead of os.path.isfile().
I suggest testing for mw_cp._manifest and removing it if it exists,
inside a finally clause hanging off the try of line 97. That way if
an exception occurs, mw_cp._manifest will still be deleted.
test_mw_with_engine.py:
57: os.path.exists() instead of os.path.isfile()
test_mw_without_engine.py:
59: os.path.exists() instead of os.path.isfile()
Thanks,
Jack
On 07/18/12 06:01 AM, Nirmal Agarwal wrote:
Hi all,
Can I please get review for CR 7179899.
7179899 lib/install_manifest/test leaves
/tmp/test_manifest_writer_01_z1YC7Y.xml behind after running tests
webrev :
https://cr.opensolaris.org/action/browse/caiman/nirmal27/7179899/webrev/
Testing :
Verified that /tmp/test_manifest_writer_01* doesn't exist after
running the tests.
Pep8 is clean.
Pylint : removed some of unused imports.
Thanks,
Nirmal
_______________________________________________
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
