Hello, Srirama and Paul,

Thanks for your comments.
I have updated code according to your comments.
Webrev: http://cr.opensolaris.org/~heli/httping/

Best Regards,
Heli

Srirama Sharma wrote:
> Hi Heli,
>
> Below are few comments
>
> 1. There are few fields (NAME and SRC) missing in METADATA, please 
> follow the rules specified in the link below : 
> http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines
>
> 2. usr/src/cmd/httping/Makefile.sfw
>     - VERSION and PKGNAME information could be obtained from the 
> METADATA file as specified below, instead of hard coding the same in 
> Makefile.sfw
>          PKGNAME=$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh)
>     - Please change 'env ' to 'env - '
>     - you could split the target 'all' into two. One target to prepare 
> the source (i.e untar and patching) and the other target to build (i.e 
> to do make)
>       something like:
>
>             all: src
>                  env - \
>                  ....
>                  ....            
>                  (CCSMAKE) -e all)          
>
>             src: $(TARBALL)  
>                    gzip -dc $(TARBALL) | tar xopf -
>                    (cd $(PKGNAME); \
>                    $(GPATCH) -p0 < ../httping-makefile.patch;)
>
>     - If you have added "-L/usr/sfw/lib" and "-I/usr/sfw/include" into 
> LDFLAGS and CFLAGS  to look for libssl and libcrypto, then it can be 
> removed as they are now present in "/usr/lib".
>
> 3.    usr/src/cmd/httping/install-sfw
>     - In line 38, you could pass on PKGNAME variable to install-sfw 
> from Makefile.sfw instead of hard coding the VERSION.
>     - you could move line 44 into Makefile.sfw only i.e you can patch 
> the manpage in the same target where you would patch Makefile. (in the 
> target src as specified in (2) )
>     - please use ksh93 as specified below     
>
>     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)
>
>
> 4. usr/src/pkgdefs/SUNWhttping/copyright
>      - The disclaimer section is missing. It should be put in the 
> beginning followed by the Copyright statements and then the GPL 
> license text.
>
> 5. usr/src/pkgdefs/SUNWhttping/depend
>      - The Copyright statement has to be moved after the 'CDDL HEADER 
> END' .
>      - specifying SUNWopensslr in the depend file is sufficient. you 
> can remove "SUNWopenssl-libraries".
>      - Please check if there are any missing dependencies by running 
> the check-deps.pl script or by doing "make check_deps" in 
> usr/src/pkgdefs/SUNWhttping dir.
>    
> 6. usr/src/pkgdefs/SUNWhttping/pkginfo.tmpl
>     - Please add the version in the DESC. Something like :
>       DESC="httping - ping for http-requests (1.2.9)"
>
> Also note that you will have to get a contract to use libssl and 
> libcrypto before ARC review.
>
> Thanks,
> Srirama
>
>
> Heli said the following on Friday 13 February 2009 09:03 AM:
>> Hello, All,
>>
>> Please review the codes for httping porting.
>> Webrev:  http://cr.opensolaris.org/~heli/httping/
>>
>>
>> Httping is like 'ping' but for http-requests. Give it an url, and 
>> it'll show you how long it takes to connect, send a request and 
>> retrieve the reply.
>>
>> Best Regards,
>> Heli
>>
>>
>>
>> _______________________________________________
>> sfwnv-discuss mailing list
>> sfwnv-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss

Reply via email to