Looks good.
Jean
Karen Tung wrote:
> Hi Jean,
>
> Thank you for the code review. My responses inline.
>
> Jean McCormack wrote:
>> DC_defs.py:
>>
>> For consistency sake it would be nice to see you do this:
>> PKGS_TO_INSTALL = IMG_PARAMS + "/packages"
>> PKGS_TO_UNINSTALL = IMG_PARAMS + "/post_install_remove_packages"
>> PKG_NAME = "/pkg/name"
>> PKG_NAME_UNINSTALL = PKGS_TO_UNINSTALL + PKG_NAME
>> PKG_NAME_INSTALL = PKGS_TO_INSTALL + PKG_NAME
>>
>> or something like that.
> Great idea. This is indeed much cleaner.
>>
>>
>> DC_tm.py:
>> line 275: If you changed the above this would be easier to read and
>> more consistent
>> with the rest of the code as:
>>
>> pkgs = get_manifest_list(manifest_server_obj, PKG_NAME_INSTALL)
> Changed as suggested.
>>
>> line 315: similar to above.
> Changed as suggested.
>>
>> transfer_mod.py:
>> I'm not fond of using 0 and 1 to flag install or uninstall. What
>> about just passing in the action_str?
>> So the call becomes perform_ips_pkg_opt("install") or
>> perform_ips_pkg_opt("uninstall") and you
>> can remove lines 1008-1011.
>>
> Great idea. Also changed as suggested. I replaced lines 1008-1011
> with a check to
> make sure that the string passed in is either "install" or "uninstall"
> and nothing else.
>
> The updated webrev is here for your reference:
>
> http://cr.opensolaris.org/~ktung/dc_uninstall_changed/
>
> Thanks again for the review.
>
> --Karen
>> Jean
>>
>> Karen Tung wrote:
>>> Please review my changes for the following 2 bugs:
>>>
>>> 3398 slim_install should be removed from the CD during finalization
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=3398
>>>
>>> 3812 DC documentation
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=3812
>>>
>>> webrev:
>>> http://cr.opensolaris.org/~ktung/dc_uninstall/
>>>
>>> Please don't worry about reviewing the content of the
>>> distro_const.1m man page
>>> and all the HTML files. Those are just place holders for now.
>>> Barbara will
>>> update them in the next couple of weeks.
>>>
>>> Thanks,
>>>
>>> --Karen
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> caiman-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>
>>
>
