Hi Vladimir,

Just a couple of minor comments.

Vladimir Marek wrote:
>
> I found one bad year, and one expanded SCCS keywords. Am I missing
> something else ?
>   

I'll be happy with those changes. There are a few extra lines here and 
there, but all cosmetic really.
>
> Yes. unrar does not have man page. I created one (Linux has man page,
> but it's not up to Solaris standards really) and offered it to unrar
> authors. The man page probably will be in next unrar release.
>   
Does it really need the unrar copyright info then? Sun supplied man 
pages these days have CDDL headers as well. See lighttpd14 for a couple 
of examples.

Otherwise it looks good.

Amanda
>
>   
>> Does the  original include the license information at the end as well?
>> (the rarlab  website just broke so I can't check). I think what you've
>> done is ok,  maybe others might comment.
>>     
>
> At the moment there is no official man page, but in future mine might
> become one.
>
>
>
>
>   
>> usr/src/pkgdefs/SUNWunrar/depend
>> - You don't need to include this in your workspace, in your Makefile you 
>> have DATAFILES= depend and that will copy in the standard workspace  
>> depend file during the build
>>     
>
> Ah, right
>
>
>
>   
>> usr/src/cmd/unrar/install-unrar
>> - Use ksh93 instead of sh. The following describes the 3 things you need 
>> to do:
>>
>>     
>>> use /usr/bin/ksh93 or /usr/bin/bash for install-sfw*
>>> and add a $ set -o errexit # at the beginning and
>>> replace ". ${SRC}/tools/install.subr" with
>>> "source ${SRC}/tools/install.subr" (the idea is to
>>> catch failures in the script and abort it at that
>>> point, right now the script will just continue)
>>>       
>
> Done
>
>
>   
>> usr/src/cmd/unrar/sunman-stability
>> - Change 'expect' to 'unrar' in "Source for expect is available on  
>> http://opensolaris.org.\";
>>     
>
> Uh,oh :) Done
>
> Fixed webrev is again at http://cr.opensolaris.org/~neuron/unrar
>
> Thank you
>   


Reply via email to