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
>