D'oh I see I missed lot's o' folks input already for the actual code
review and my sed should probably have been: "sed
's/^flush-content-cache$/flush-content-cache-on-success = True/'" or
whatever the full string is as Sundar said.
Thank you,
Clay
On Mon, 23 Jun 2008, Clay Baenziger wrote:
> Hi Karen,
> As I see it, I think this is an okay interim fix but the final
> resolution should be implementing a postinstall action using Jan's
> framework from bug 1040 to flip this setting in on the installed system
> only.
> That way downloading a package while running from the LiveCD won't
> require using a lot more memory space to hold cache files which won't be
> needed for a LiveCD environment, and the file can still get updated for
> the installed environment.
>
> As far the code review:
>
> Lines 57-59: it seems the sed(1) line is more complex than necessary, but
> I'm not as familiar with the /var/pkg/cfg_cache file as I'd like. Why not:
> sed 's/flush-content-cache/flush-content-cache-on-success = True/'
> $cfg_file > $tmp_cfg ?
>
> Line 63 why not set $status and return that rather than whatever "pkg
> image-create" returned on line 38?
>
> Lines 66-67 why not consolidate as a mv(1) like 'mv $tmp_cfg
> $cfg_file' and then check its status too like lines 61-64?
>
> Thank you,
> Clay
>
> On Mon, 23 Jun 2008, Karen Tung wrote:
>
> > Hi,
> >
> > Please review my changes for bug 2263: microroot overflows with latest
> > changes to IPS
> >
> > http://defect.opensolaris.org/bz/show_bug.cgi?id=2263
> >
> > webrev is at:
> >
> > http://cr.opensolaris.org/~ktung/2263_fix/
> >
> > Thanks,
> >
> > --Karen
> >
> > _______________________________________________
> > caiman-discuss mailing list
> > caiman-discuss at opensolaris.org
> > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
> >
>
