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.

Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to