Brock Pytlik wrote: > Shawn Walker wrote: >> Brock Pytlik wrote: >>> 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. >> >> This was done on purpose. >> >> My reasoning was as follows: >> >> --log-access and --log-errors have three special values each >> indicating logging destination *explicitly* (stderr, stdout, none) and >> anything else is assumed to be a filename if present. >> >> An empty string creates ambiguity in the command-line parsing for this >> reason: >> >> ./depot.py --log-access= --readonly >> ./depot.py --log-access --readonly >> >> ...while I know that this is a side-affect of how long-options are >> parsed, it's also very annoying to me. >> >> As such, I would avoid "empty string" for the reason above and change >> --proxy-base instead to allow a value of "none" in depot.py for the >> same reason it's done for the logging parameters if you must change it. >> >> Danek and I recently discussed this topic in this thread: >> http://mail.opensolaris.org/pipermail/pkg-discuss/2008-October/007257.html >> >> >> I strongly prefer having an explicit destination value of "none" for >> the logging commands over just using an empty string since that is >> consistent with the other options of "stdout" and "stderr". >> >> Cheers, > Well, the nice thing about using this ksh script is that now the depot > never sees --log-access= or --log-access. If the option isn't set, the > flag never appears at all.
Right, but I was trying to keep the SMF usage scenario consistent with the "user using this from the command-line case" scenario. However, if this change is only going to affect the SMF scenario, I have no strong objections. Cheers, -- Shawn Walker _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
