Hi Sanjay, Thank you for the code review. Please see my responses inline.
Sanjay Nadkarni wrote: > I noticed in the diffs that that the line numbers on the old file > don't match what's in the checked in code. I am not sure why. I am not sure why either. I updated my gate to the latest... > > BC_tm.py: > 99: Could you please add doc comments for this def Done. > > def DC_ips_validate_auth > If I understand correctly, the main purpose of this function is that > if pref_flag is set, the no-refresh flag for IPS occurs by default. > If so, could this be added to the comment. No, that's not the purpose of this function. The purpose of this function is to try to validate the value that's specified for the post-install authority/mirror/additional authority. Previously, the code validate it by doing a "pkg refresh" on the package image area. Unfortunately, "pkg refresh" will modify the catalog of the package image area to the post-install authority's catalog, which is not correct. We want to keep the original catalog that we populate the package image area with. So, the DC_ips_validate_auth function will set up an "ALTERNATE package image area", so, we can do "pkg refresh" on it to validate the authorities and not worry about messing up the catalog in the real package image area. > 232: looks like a bogus change. Please add the period back. > Done > > The reset looks file to me. > Here's the updated webrev with updates from both you and Jack's comments: http://cr.opensolaris.org/~ktung/4550_fix_update/ Thanks again for the code review. --Karen > > -Sanjay > > > Karen Tung wrote: >> Hi, >> >> Can I have 2 reviews for the changes to fix bug 4550 asap? >> >> 4550 'pkg uninstall SUNWslim-utils' fails with osol-0811-101a-rc1a.iso >> http://defect.opensolaris.org/bz/show_bug.cgi?id=4550 >> >> webrev: >> >> http://cr.opensolaris.org/~ktung/4550_fix/ >> >> Thanks, >> >> --Karen >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> >
