Dan Price wrote: > On Tue 28 Oct 2008 at 02:34PM, Brock Pytlik wrote: > >> Hi, here's my first shot at fixing 2717 to make it easier to deploy new >> depos. >> >> I don't have much experience with ksh scripting (you're looking at my >> first venture) so I'm sure there are lots things I should do differently. >> > > --- > The script looks good overall, although I think you will want to add > some comments so that the next person who comes along can easily plug > in a new option with confidence. > Ok, added what I think is needed. > --- > I think you're going to want to run "exec $cmd" at the end of the start > method, since you'll want to have this script cease to exist. > Fixed > --- > Our standard shell-style tends to be: > > if [[ $? -ne 0 ]]; then > fi > > And: > > for b in $boolean_props; do > done > Fixed > > --- > When calling exit, you'll want to use the standard SMF exit codes in > place of exit 1, exit 0, etc. In particular, you can signal that a > particular error is fatal and that SMF should stop retrying, etc. > > In some places you do so, and in some place you don't. Another problem > is that you must specifically include /lib/svc/share/smf_include.sh or > those constants won't exist. > Fixed. I wasn't sure what code to use for the usage case, so I went with ERR_CONFIG as that looked better than the other choices. > --- > Since (I think) this is a ksh (and not a sh) script, run it via > /usr/bin/ksh, instead of /sbin/sh. Otherwise, it won't work on Nevada, > where /sbin/sh is bourne shell. > Ok > --- > Can you do me a favor and put the bool_ops before the option_ops? This > way, it'll be easier to pick out the "major mode" of the server, like > whether it is --mirror or whatever, from the output of pgrep. > Yep, fixed.
Brock > >> Webrev: >> http://cr.opensolaris.org/~bpytlik/ips-2717-v1/ >> >> Bug: >> http://defect.opensolaris.org/bz/show_bug.cgi?id=2717 >> pkg.depotd: need smf support for depot mode options >> >> Thanks, >> Brock >> _______________________________________________ >> pkg-discuss mailing list >> [email protected] >> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss >> > > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
