James, See below for comments ...
Paul jin wan - Sun Microsystems - Beijing China wrote: > I am porting 'dbunit', which is a JUnit extension for database-driven > projects test to OpenSolaris, > > Would you please have a review on my code. The webrev is at: > http://cr.opensolaris.org/~bjwancn/dbunit/ === Start of Comments ==== 1. usr/src/lib/dbunit/METADATA This doesn't need the CDDL HEADER stuff Add missing fields, see ... http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines 3. CDDL HEADER, file top, in various files .. Change so that they conform to those in ... "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; See ... http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines 4. usr/src/lib/dbunit/Makefile.sfw Change 'env ' to 'env - ' to make sure it only see the env variables they should really get (and not pick-up any random env variable) Extract DBUNIT= info from the METADATA, see recent integrations for examples Use predefined value for ANTHOME=/usr from Makefile.master, eg. ... ANTHOME=$(CFGPREFIX) 5. usr/src/lib/dbunit/install-sfw Pass VER= info in from Makefile.sfw as environment var or an option. Roland Mainz wrote: > 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) 6. usr/src/lib/dbunit/sunman/dbunit.3.sunman Is "Interface Stability External" correct? Add CDDL HEADER stuff Change ... "Source for junit is available ..." to "Source for dbunit is available ..." 7. usr/src/pkgdefs/SUNWdbunit/copyright Add sun disclaimer and package src copyright stuff at top see .. "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright"; 8. Pkg dependencies You are using the default 'depend', but have you checked your pkg has no other dependencies with the dependency checker script? 9. usr/src/pkgdefs/SUNWdbunit/pkginfo.tmpl Change (for consitancy) ... 42 DESC="dbunit - JUnit database extension 2.3.0" to .. DESC="dbunit - JUnit database extension (2.3.0)" 10. usr/src/pkgdefs/SUNWdbunit/prototype_com Do you need to deliver the src stuff as part of this pkg, eg. in ... usr/share/doc/dbunit/src/* Just checking? 11. usr/src/pkgdefs/SUNWdbunit/prototype_i386 & usr/src/pkgdefs/SUNWdbunit/prototype_sparc Cosmetic: add the package name comment line, etc, see ... "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/prototype_i386"; === End of Comments ====== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
