That's great, it looks fine now. Amanda
karol wrote: > Hi Amanda, > > Thanks for the comments. > See the fix here: > > http://cr.opensolaris.org/~lxin/iperf/ > > Here is the diff webrev: > http://cr.opensolaris.org/~lxin/iperf_incr/ > > ? 2009?02?10? 16:25, Amanda waite ??: >> Just some minor points: >> >> usr/src/cmd/iperf/Makefile.sfw >> - Was this file checked out when you created the webrev? the ident >> string suggests that it was, so check to make sure. > Yes, it's checked out. > > Thanks, > Karol >> >> usr/src/cmd/iperf/METADATA >> - You could use >> "http://downloads.sourceforge.net/iperf/iperf-2.0.4.tar.gz"; as the >> download URI >> >> usr/src/cmd/iperf/install.sfw >> - You should rename this to install-sfw for consistency with other >> components (obviously change it in Makefile.sfw too) >> - Tidy up the indents near the end (obvious if you look at the raw >> version of the file in the webrev >> >> All files in pkgdefs/SUNWiperf >> - Tidy up the CDDL headers according to the examples here: >> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/ >> >> Use the prototype.Makefile as an example >> >> Amanda >> >> >> * >> *karol wrote: >>> Hi All, >>> >>> Thanks for your comments. I have fixed them. >>> Please help to check the change. >>> http://cr.opensolaris.org/~lxin/iperf/ >>> >>> Here is the diff webrev: >>> http://cr.opensolaris.org/~lxin/iperf_incr/ >>> >>> The patch file is removed since the issue can be resolved via >>> passing arguments to ./configure. >>> >>> Thanks, >>> Karol >>> >>> ? 2009?02?09? 17:08, Srirama Sharma ??: >>>> Hi Karol, >>>> >>>> Below are few comments >>>> >>>> 1. SOURCE field is missing in the METADATA file.please follow the >>>> rules specified in the link below : >>>> http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines >>>> >>>> 2. In Makefile.sfw, >>>> - please use "env -" instead of "env" throughout. >>>> - you could call configure as "$(SHELL) ./configure" so that there >>>> will be predictability of which shell will be called. >>>> - Also it appears that you are patching the source base after doing >>>> a configure. It should generally be other way round. i.e untar the >>>> tarball, patch it, run configure and then do a make >>>> >>>> 3. In install-sfw, >>>> - In line 43, man page shouldn't be installed as a normal file. >>>> Instead please use 'M' (as shown below) so that the >>>> sunman-stability gets appended to the manpage. >>>> _install *M* ${PKGDIR}/man/iperf.1 ${MAN1DIR}/iperf.1 444 >>>> >>>> 4. iperf-2.0.4-pthreads-rt.patch >>>> - Since Makefile gets generated during configure, any changes to it >>>> should be made into Makefile.in/Makefile.am. >>>> - Again, you shouldn't be editing "config.h" file which gets >>>> generated after doing a configure. Instead patch "config..h.in" file. >>>> >>>> 5. In depend file, >>>> - Please move the Copyright lines after the CDDL HEADER END. >>>> - you could use the default depend file here if there are no >>>> project specific dependencies. Please check. >>>> >>>> 6. In pkginfo.tmpl, you could add the version in the DESC field as >>>> below >>>> DESC="iperf - tool for measuring maximum TCP and UDP bandwidth >>>> performance. *(2.0.4)*" >>>> >>>> Thanks, >>>> Srirama >>>> >>>> karol said the following on Monday 09 February 2009 11:58 AM: >>>>> Hi All, >>>>> >>>>> I'm working on porting package "iperf" to opensolaris, which is for >>>>> measuring max TCP/UDP bandwidth performance. >>>>> >>>>> Could you help to review it? >>>>> >>>>> http://cr.opensolaris.org/~lxin/iperf/ >>>>> <http://cr.opensolaris.org/%7Elxin/iperf/> >>>>> >>>>> Thanks, >>>>> Karol >>>>> _______________________________________________ >>>>> sfwnv-discuss mailing list >>>>> sfwnv-discuss at opensolaris.org <mailto:sfwnv-discuss at opensolaris.org> >>>>> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss >>>>> >>> >> >
