Hi Paul & Amanda, Thanks very much of your review, and please see my answers to both of your comments in lines.
I sync the webrev at the same location: http://cr.opensolaris.org/~bjwancn/dbunit/ Thanks! -James On 02/20/09 17:20, Paul Cunningham wrote: > > === 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 > Fixed > 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 > Fixed > 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) > Fixed > 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) > Fixed > 6. usr/src/lib/dbunit/sunman/dbunit.3.sunman > Is "Interface Stability External" correct? > Changed "External" to "Uncommitted" > Add CDDL HEADER stuff > It seems no need to add CDDL HEADER in sunman file according to "http://tas.sfbay/net/sfwnv.sfbay/gates/sfwnv/gate/usr/src/cmd/expect/sunman-stability"; > Change ... > "Source for junit is available ..." > to > "Source for dbunit is available ..." > Fixed > 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"; > > > Fixed > 8. Pkg dependencies > You are using the default 'depend', but have you checked > your pkg has no other dependencies with the dependency > checker script? > Add a depend file > 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)" > Fixed > 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? > usr/share/doc/dbunit/src/* is the sample test code for dbunit, not the src of dbunit itself. > 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"; > > > Fixed > === End of Comments ====== On 02/20/09 17:21, Amanda waite wrote: > usr/src/lib/dbunit/METADATA* > * - No need for a CDDL header in the METADATA file > - Add a Name field as per the METADATA section of > http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines > - Add other fields (SRC, SUPPORT, BUGTRAQ) > Fixed > usr/src/lib/dbunit/Makefile.sfw > - Set DBUNIT with values from the METADATA file: > $(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) > - In the 'all' target use 'env -' in place of 'env' > - In the 'install' target', replace $(SH) with $(SHELL) <- which is ksh93 > Fixed > usr/src/cmd/mpg123/install-sfw > - add 'set -o errexit' as the first line after the ident string > - Pass in 'VER' from Makefile.sfw, i.e.: VER=$(DBUNIT) $(SHELL) > ./install-sfw > Fixed, I got "VER" from METADATA. > usr/src/lib/dbunit/sunman/dbunit.3.sunman > - Interface Stability is External. Where did this come from? External > is now Volatile, is there an ARC case for DBUnit? > Fixed, see above > usr/src/pkgdefs/SUNWdbunit/prototype_com > - Just use the package name as the final comment instead of > "SUNWdbunit - JUnit database extension" > > usr/src/pkgdefs/SUNWdbunit/prototype_sparc, > usr/src/pkgdefs/SUNWdbunit/prototype_i386 > - Add the following as the final comments for consistency > > # source locations relative to the prototype file > # > # > # SUNWdbunit > # > Fixed > > CDDL headers for all files > - Make sure all the CDDL headers and Copyright notices conform to: > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/ > Fixed > Amanda
