William, See below for my comments from my quick skip through ...
Paul On Tue, 2008-07-15 at 22:05, William Schoofs wrote: > I have posted a webrev for the package "libmikmod" which I am porting to > OpenSolaris and would like to request a code review. > > Please see http://cr.opensolaris.org/~wjs/libmikmod/webrev/ and provide any > comments as needed if there are any issues which I need to correct. ==== Start of comments ==== 1. usr/src/pkgdefs/Makefile You need to resysnc this with the gate so it doesn't look like you are trying to delete stuff. 2. usr/src/pkgdefs/SUNWlibmikmod/depend Have you run the pkg dependencies script on your package? Move the 'Copyright' lines to after the 'CDDL HEADER END' header 3. usr/src/pkgdefs/SUNWlibmikmod/prototype_sparc You have the line ... 'd none usr/bin/sparcv9 755 root bin' in it twice. And same for usr/src/pkgdefs/SUNWlibmikmod/prototype_i386 4. usr/src/lib/libmikmod/METADATA Add line for URL to where you got source code tarball from. 5. usr/src/lib/libmikmod/Makefile.sfw Change so you use the predefined '--prefix=' from Makefile.master, eg. something like .. CONFIGURE_OPTIONS += --enable-shared CONFIGURE_OPTIONS += --disable-static ..... ./configure $(CONFIGURE_OPTIONS) There should be various examples of this in the gate now, eg. cups. Combine the two '-rm -rf ...' lines into one. Rather than 'all: real-all' & 'real-all: all32 all64' why not just .. 'all: all32 all64' 5. usr/src/lib/libmikmod/install-libmikmod Do you need 'chmod 644 ${INFODIR}/dir'? Shouldn't it be in Targetdirs with the correct permissions? 6. usr/src/lib/libmikmod/libmikmod-3.2.0-beta2.tar.gz Beta version? Is it okay to use this. 7. Everything else looks okay to me. ==== End of comments ====== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Computer Products
