Chris, See below for my comments ...
Paul Chris Liu wrote: > > Please help code review the inclusion of libsndfile-1.0.17 into the sfw > consolidation. It is an audio data reading/writing library. > its website is at > http://www.mega-nerd.com/libsndfile/ > > The webrev is at: > http://cr.opensolaris.org/~chrisliu/SNDFILE > > libsndfile-1.0.17 has problem with latest version FLAC (1.2.1) > therefore I added a patch here === Start of Comments === 1. usr/src/lib/libsndfile/METADATA You might want to add a 'URL:' line, see .. "http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines"; 2. usr/src/lib/libsndfile/Makefile.sfw Is "PREFIX=/usr" (line 30) used? If not delete it You could extract the 'VER =' & 'TARBALL =' from the METADATA something like .. > VER =$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) > TARBALL =$(VER).tar.bz2 Could you have built this using the Sun Compiler (rather than gcc)? I think that is the preferred way! 3. usr/src/lib/libsndfile/install-sfw The directories created by 'mkdir -p' should be in the 'Targetdirs' file (and removed from here). 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) There is no sccs ident line in this file 4. usr/src/lib/libsndfile/sunman-stability SUNWlibnet ??? Id the 'Stability' level correct? 5. usr/src/pkgdefs/SUNWlibsndfile/Makefile It doesn't look as though the sccs ident line is set up correctly! 6. usr/src/pkgdefs/SUNWlibsndfile/copyright Do you need to put the full licence in here - most people do now - so have you checked. 7. usr/src/pkgdefs/SUNWlibsndfile/depend sccs ident line ? check it's set up correctly - it looks as though you just copied the expanded one from where-ever. Have you checked you have no other dependencies with the dependency checker script? Move the 'Copyright lines to after the "CDDL HEADER END" header. 8. usr/src/pkgdefs/SUNWlibsndfile/pkginfo.tmpl sccs ident line ? You might want to put the pkg version at the end of the DESC line .. "DESC="......... (1.0.17)" 9. usr/src/pkgdefs/SUNWlibsndfile/prototype_com + usr/src/pkgdefs/SUNWlibsndfile/prototype_i386 + usr/src/pkgdefs/SUNWlibsndfile/prototype_sparc sccs ident line ? === End of Comments ===== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
