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
>>
>
