Hi Paul, Thanks for your review, response in line ...
Paul Cunningham wrote: > Ivan, > > See below for my comments from my quick skip through ... > > Paul > > Ivan shi wrote: > >>>> >>>> I am porting "iozone", a filesystem benchmark tool. >>>> >>>> Could you please help to review the changes? The webrev is at: >>>> http://cr.opensolaris.org/~ivanshi/iozone/ > > .. cut .. > >> I have changed all according to your comments. >> Please reload the above link. > > 1. usr/src/cmd/Makefile > & usr/src/pkgdefs/Makefile > This needs resyncing with the gate so it doesn't > look as you are trying to change other stuff. > OK, sync'ed. > 2. usr/src/cmd/iozone/METADATA > Lines ... > 4 VERSION: 3_321 > 11 SRC: http://www.iozone.org/src/current/iozone3_315.tar > the versions differ! > Changed. > 3. usr/src/cmd/iozone/iozone3_321.tar > Maybe you should compress this with bzip2 before > putting into your ws so it takes up less space. > Done. > 4. usr/src/cmd/iozone/Makefile.sfw > Change as .. > Roland Mainz wrote: > > use "env - ..." and not "env ..." in the Makefiles to > > make sure "configure"&&"make" only see the env variables > > they should really get (and not pick-up any random env > > variable) > > Extract the VER= info from the METADATA. > You could then also add TARBALL= and use that in place > of '$(VER).tar'; see example in .. > "http://src.opensolaris.org/source/xref/sfw/usr/src/cmd/cups/Makefile.sfw"; > > OK, use TARBALL > 5. usr/src/cmd/iozone/install-iozone > Rename this file to install-sfw its more common name. > Done. > 6. usr/src/pkgdefs/SUNWiozone/copyright > Lines 11/12 say ... > 11 This "iozone v. 3_321" open source technology contains > several files under > 12 various licenses. > so should you be including the full versions of all those > licences here ? > Added. > 7. usr/src/pkgdefs/SUNWiozone/prototype_com > You are delivering ... > 52 f none usr/bin/fileop > so should you also be delivering a man-page for it? > As iozone has been linked to fileop.o object, the program is useless. I removed it. Please reload the link. Thanks, Ivan
