Fixed as noted below. Unfortunately, I tested this on ipkg, and ksh88 doesn't have associative arrays. Here's a new webrev which deals with this reality. Webrev: http://cr.opensolaris.org/~bpytlik/ips-2717-v3/
Dan Price wrote: > On Tue 28 Oct 2008 at 05:49PM, Brock Pytlik wrote: > >> Updated webrev at >> http://cr.opensolaris.org/~bpytlik/ips-2717-v2/ >> >> It removes refresh-index and rebuild from the smf property list. It also >> cleans up a few typos in the ksh script and adjusted the man pages to >> reflect the changes. >> > > Looks pretty good and much improved from last go round. > > I found it a little confusing that $trans is used for short_option_props > and for long_option_props, but not for boolean_props. Perhaps for > consistency, it'd be better (and easier to maintain) if boolean props > were also passed through the $trans transformation? > Good point, that's changed. > Nuke comments that say '# Check that svcprop worked correctly.' -- your > audience knows what $? means. > Ok. One of those rare times when the audience knows more than the writer ;) > Why is it that we're testing properties against "none" versus just being > set to the empty string? It seems like it will work, but I don't really > get the idiom. > I went with none because it was the default/empty value for several properties already. I'm much happier with checking the empty string, but 'none' seemed to be the consensus in the manifest. I'll change it and the manifest to use empty strings instead of none and see if there are problems or complaints about it. > In several places (64, 79, etc): it's --> its. > Ack, thanks for catching that, and the review in general. Brock > -dp > > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
