Evgeny,

See below ...

Evgeny Bessonov wrote:
> 
> thanks a lot for your review.
> 
> Paul > 1. usr/src/cmd/ant/METADATA: missing lines
> - Done
> 
> Paul > 2. usr/src/cmd/ant/Makefile.sfw: env - \
> - Done

What about extracting the NAME/VERSION from the METADATA ?

> Paul > 3. usr/src/cmd/ant/install-ant: ksh93, set -o, etc
> - Done
> 
> Paul > 4. usr/src/pkgdefs/SUNWant/prototype_com: 0444 for some files
> - Done

You probably should have changed them in install-ant also

> Jim > rename ant.1.sunman to sunman-stability,
> Jim > and move it up to /src/cmd/ant, and
> Jim > remove sunman directory, and update
> Jim > install-sfw accordingly.
> Paul > I'm not sure you needed to rename ant.1.sunman to
> Paul > sunman-stability because your file looks like the actual
> Paul > man page rather than a modification to an existing one
> Paul > (which is what sunman-stability is usually used for).
> 
> Jim, Paul, what is conclusion?
> For now I did everything in accordance with the first suggestion:
> sunman-stability in /src/cmd/ant
> Should I change it back?

What ever you decide is okay with me. But if the content is a full man 
page, then the name sunman-stability may confuse others (well me).

> Updated webrev is available here (please press Refresh in your browser):
> http://cr.opensolaris.org/~jinb/ant/

Everything else looks okay to me
Paul


> Paul Cunningham wrote:
> 
>> See below for some comments ...
>>
>> Paul
>>
>> Evgeny Bessonov wrote:
>>  .. cut ..
>>
>>>
>>> There is updated webrev at opensolaris.org:
>>> http://cr.opensolaris.org/~jinb/ant/
>>
>>
>> === Start of Comments ===
>>
>> 1. usr/src/cmd/ant/METADATA
>>    Add missing lines, see the wiki guidelines page ...
>>    http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines.
>>
>> 2. usr/src/cmd/ant/Makefile.sfw
>>    You could extract the VER= stuff from the METADATA content,
>>    something  like ...
>>      VER =$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh)
>>      TARBALL =$(VER)-src.zip
>>
>>   Change 'env \' to 'env - \' ...
>>     Roland Mainz wrote: to make sure it only sees the env variables
>>     they should really get(and not pick-up any random env variable)
>>
>> 3. usr/src/cmd/ant/install-ant
>>    Roland Mainz wrote:
>>    > 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)
>>
>>    I'm not sure you needed to rename ant.1.sunman to
>>    sunman-stability because your file looks like the actual
>>    man page rather than a modification to an existing one
>>    (which is what sunman-stability is usually used for).
>>
>> 4. usr/src/pkgdefs/SUNWant/prototype_com
>>    Do the installed files need the 'write permission' bit set,
>>    if not remove it.
>>
>> === End of Comments =====

-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Business Unit

Reply via email to