Hi Xiao, Without looking at anyone else's comments, see below for my comments on the updated webrev (mainly looks good to me) ...
Paul xiao li - Sun Microsystems - Beijing China wrote: > > updated webrev: > http://cr.opensolaris.org/~xl222276/sg3utils/ === Start of Comments === 1. usr/src/cmd/sg3_utils/METADATA Make the NAME: field more descriptive, eg. what is sg3 ? 2. usr/src/cmd/sg3_utils/Makefile.sfw Do you need the lines .. 53 find $(VER) -type d -exec /usr/bin/chmod 755 "{}" \; 54 find $(VER) -type f -exec /usr/bin/chmod ugo+r "{}" \; if not remove them Change line ... 48 ./configure --prefix=/usr) so it uses the predefined prefix in Makefile.master to something like .. ./configure $(CONFIGURE_OPTIONS)) 3. usr/src/cmd/sg3_utils/install-sfw Change .. 1 #! /usr/bin/ksh to .. #! /usr/bin/ksh93 Have you run '/usr/bin/ksh93 -n install-sfw' on the script to ensure it doesn't show up any errors or warnings? Remove the double-space in the CDDL header (check other files to), eg. change # .... to # .... 4. usr/src/pkgdefs/SUNWsg3utilsr/copyright & usr/src/pkgdefs/SUNWsg3utilsu/copyright Do you need to put the full licence in here ? (I prefer it as it is though) 5. usr/src/pkgdefs/SUNWsg3utilsr/pkginfo.tmpl You could change DESC= to .. DESC="sg3 - package of utilities for sending SCSI commands 1.25 (root)" 6. usr/src/pkgdefs/SUNWsg3utilsu/depend Have you checked you have no other dependencies using the dependency checker script? 7. usr/src/pkgdefs/SUNWsg3utilsu/pkginfo.tmpl You could change DESC= line as in (5) === End of Comments ===== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
