Hi Heli,

The webrev looks good to me.

Thanks,
Srirama

Heli said the following on Monday 16 February 2009 12:06 PM:
> 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