Steven,
I took another quick look - you might also want to change the following ...
1. usr/src/cmd/bonnieplus/METADATA
Add the BUGTRAQ and SUPPORT info, eg. ...
SUPPORT: ????
BUGTRAQ: solaris/utility/????
2. usr/src/cmd/bonnieplus/install-sfw
(I forget if you responded to this ..)
You might want to apply these comments ..
Roland Mainz wrote:
> - Please 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)
3. usr/src/cmd/bonnieplus/Makefile.sfw
(again I forget if you responded to this ..)
You might want to apply these comments ...
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)
> - use either $(SHELL) or /usr/bin/bash for "configure"
> calls (so we know which one is used and "configure"
> doesn't pick one itself)
Christopher Mi wrote:
> Use the method define in Makefile.master
> since you have a standard METADATA file.
>
> VER =$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh)
> TARBALL =$(VER).tar.bz2
Does it really core dump during the 'all:' rule, if not
remove the 'find ... core ...'. If it does, why does it?
4. depend
Did you check you have no other dependencies?
Everything else looks okay to me
Paul
Steven Le wrote:
> Hi all,
> I have fixed all the problems that you suggested me to do. The up2date
> files are at:
> http://cr.opensolaris.org/~steven84/bonnieplus/
>
> Please email if there is anything
> Note: We need your final approval in order to put back, please help me
> as soon as you can.
>
> 1st review: FIXED
>
> * Create webrev with the CR#6741736 FIXED
> * usr/benchmarks needs to added as part of usr/src/Targetdirs FIXED
> * in METADATA mention URL of bonnie++ FIXED
> * bonnie++ should be installed in usr/sbin instead of usr/bin FIXED.
> NOTE: our sponsors request us to put into usr/benchmarks
> * zcav is missing in prototype_com and also add manpage FIXED
> * in usr/src/pkgdefs/SUNWbonnieplus/prototype_com add symlinks for
> bon_csv2txt bon_csv2html FIXED
> * in usr/src/pkgdefs/SUNWbonnieplus/prototype_sparc add '#' between
> copyright and make sure ident string is correct FIXED
> * remove usr/src/pkgdefs/SUNWbonnieplus/pkginfo file FIXED
>
>
> 2nd review: FIXED
> === Start of Comments ===
>
> 1. usr/src/cmd/bonnieplus/METADATA
> Add line for where the source code came from. 'URL: ...'
>
> 2. usr/src/cmd/bonnieplus/install-sfw
> You may want to apply 'Roland Mainz' comments, on other
> people's install-* scripts to this. You can find those
> comments in the sfwnv-discuss archives.
>
> BINDIR= is not used so you could delete
>
> 3. usr/src/cmd/bonnieplus/Makefile.sfw
> You may want to use "env - ..." rather than "env ..."
> so its doesn't pick-up any random env variable.
>
> 4. usr/src/pkgdefs/SUNWbonnieplus/Makefile
> Add line space after '#ident ...' line
>
> 5. usr/src/pkgdefs/SUNWbonnieplus/depend
> This looks like the default 'depend' file; if you have
> no other dependencies you should remove this file and
> add 'DATAFILES= depend' to your SUNWbonnieplus/Makefile.
> But have you checked you have no other dependencies with
> the dependency checker script?
>
> 6. usr/src/pkgdefs/SUNWbonnieplus/pkginfo
> Remove this files as it is generated from the pkginfo.tmpl
> file.
>
> 7. usr/src/pkgdefs/SUNWbonnieplus/prototype_sparc
> Add line space before '#ident ...' line
>
> 8. usr/src/pkgdefs/SUNWbonnieplus/prototype_i38
> & usr/src/pkgdefs/SUNWbonnieplus/prototype_sparc
> '#ident ...' line says ...
> # ident "@(#)pkginfo.tmpl ...
>
> === End of Comments =====
>
--
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Computer Products
General Dynamics Itronix Europe Ltd.
