Hi Paul, thanks for thorough review. I added some comments below and updated the webrev at http://cr.opensolaris.org/~lukas/pwgen/ . Let me know, please, if you have any additional comments.
Thanks, Lukas Paul Cunningham wrote: > Lukas, > > See comments below ... > > Paul > > Lukas Rovensky wrote: >> >> I am porting pwgen (an easy to memorize passwords generator, [1]) to >> OpenSolaris. I would like to ask for a code review. The changes can >> be found at: >> >> http://cr.opensolaris.org/~lukas/pwgen/ > > 1. usr/src/cmd/pwgen/install-sfw > Pass the VERS= info in from the Makefile.sfw > Fixed. > Change as follows ... > 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) > Changed. > 2. usr/src/cmd/pwgen/Makefile.sfw > You could add after the VER= line ... > TARBALL=$(VER).tar.gz > then replace throughout "$(VER).tar.gz" with $(TARBALL) > Added. > 3. usr/src/pkgdefs/SUNWpwgen/copyright > Add the source owner copyright lines (after the Sun disclaimer) > extracting them from the uncompressed src tarball, see ... > "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright"; > > Added. > > 4. usr/src/pkgdefs/SUNWpwgen/depend > This has the Sun Copyright lines twice, delete the > ones at the top > Fixed. > This also looks like the default set of dependencies, have > you checked the pkg has no others with the gate's pkg > dependency checker script? If if has no others then delete > this and use the default depend. > Yep, I used the dependency checker script. I compared the depend for pwgen with the one at pkgdefs/common_file and they differ, so I left the depend file as it is. > 5. usr/src/pkgdefs/SUNWpwgen/pkginfo > Delete this as its generated from the SUNWpwgen/pkginfo.tmpl > Deleted. > 6. usr/src/pkgdefs/SUNWpwgen/pkginfo.tmpl > On the DESC= line replace ', version 2.06' with ' (2.06)' > Done. > Fix the CDDL header and top of file as per ... > "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; > (there are other files that need fixing also). > Fixed (hopefully in all files).
