Hi Roland.
Thanks for your review.
I realized I pushed quickly in order to get this in today. I will make
a note of your comments in a bug we have open to clean up shell scripts
(3471) so that it is not lost.
I checked that you are already on the cc list for that bug.
Thanks,
Jack
On 11/14/08 16:17, Roland Mainz wrote:
> Jack Schwartz wrote:
>
>> On 11/14/08 15:22, Karen Tung wrote:
>>
>>> Jack Schwartz wrote:
>>>
>>>> Wow! How cool is that? Thanks, Dave!
>>>>
>>>> I've basically taken this, and indented the code between the
>>>> single-quotes.
>>>>
>>>> I tested doing a build, and also verified via a test script that the
>>>> Hard Disk entry (entries) get removed wherever they are in the menu.
>>>>
>>>> Webrev updated at the same location:
>>>>
>>>> http://cr.opensolaris.org/~schwartz/081112.1/webrev/
>>>>
>>>> On the caiman-discuss IRC channel, Dave has already approved this
>>>> change, so I need only one more reviewer.
>>>>
>>> I think we should check the return value of nawk. What if it fails,
>>> we don't want to
>>> just overwrite the file. Everything else is fine.
>>>
>> OK. I've revised the webrev. Now, if nawk fails, it prints a warning
>> but continues with the original Hard Disk entry left in. I didn't think
>> a nawk error here was worthy of trashing the whole usb image.
>>
>> Webrev is in the same place.
>>
>> I've tested error case (simulated by a bad file passed to nawk), a good
>> case, and a case where there was no Hard Disk entry in menu.lst.
>>
>
> One thing which directly jumps into my eye:
> -- snip --
> if [[ $? == 0 ]] ; then
> -- snip --
> ... please change this to:
> -- snip --
> if (( $? == 0 )) ; then
> -- snip --
>
> The differene is that [[ ]] compares strings while (( )) is an
> arithemtric expression and avoids an extra integer--->string conversion
> pass (the variable "?" is an integer by default). Beyond that it would
> be nice to make sure the new changes do not generate more hits in the
> output of $ ksh93 -n <scriptname> # ...
>
> ----
>
> Bye,
> Roland
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20081114/6cd45237/attachment.html>
