Hi, See below for my comments ....
Paul Li Fan wrote: > > Thanks for your comment. I've change the Makelife.sfw and re-generate > webrev. > > http://cr.opensolaris.org/~fan.li/aalib >> >> Li Fan wrote: >>> >>> I will integrate AA-lib 1.4rc5 into Opensoalris. The webrev is at: === Start of Comments ==== 1. usr/src/Targetdirs You do not seem to have changed anything in this file; please unedit so it doesn't show up in the webrev 2. usr/src/lib/Makefile I know the list is not in alphabetical order, but please put your entry 'aalib' at or near the top of the list (rather than at the bottom). 3. usr/src/lib/aalib/METADATA Make the NAME: field text more descriptive You can leave the COMMENT: field blank Shouldn't the URL: point to ... "http://aa-project.sourceforge.net/aalib/"; 4. CDDL HEADER and file top (various files) Cosmetic: Please change so that this conforms to that in .. "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; See http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines its probably just the missing line space after the CDDL header (but check). 5. usr/src/lib/aalib/Makefile.sfw Explicitly define the 'configure --prefix=...' value using .. ./configure $(CONFIGURE_OPTIONS) where CONFIGURE_OPTIONS is defined in Makefile.master Lines ... 80 -rm -rf $(VER) 81 -rm -rf $(VER64) combine into a single line, eg ... -rm -rf $(VER) $(VER64) Do you need this line ? ... 83 install_h: Do you need this line ? ... 31 MAKE=/usr/bin/make 6. usr/src/lib/aalib/install-sfw & usr/src/lib/aalib/install-sfw-64 Remove the write permission bit from files installed into /usr 7. usr/src/lib/aalib/install-sfw-64 Does this work in a script file ? ... 31 VER=$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh)-64 You can just pass it in from the Makefile.sfw, eg. VER=${VER64} 8. usr/src/pkgdefs/SUNWaalib/copyright Add the Sun disclaimer and the pkg src owner copyright lines as in ... "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright"; 9. usr/src/pkgdefs/SUNWaalib/pkginfo This file is generated from your SUNWaalib/pkginfo.tmpl so should not be in your webrev (remove it) 10. usr/src/pkgdefs/SUNWaalib/prototype_com & usr/src/pkgdefs/SUNWaalib/prototype_i386 & usr/src/pkgdefs/SUNWaalib/prototype_sparc Remove the write permission bit from files installed into /usr (also see (6)) === End of Comments ====== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
