Hi Jim, Could you please review my code? Until now, no one else except Paul review it.
? 12/15/08 19:14, Paul Cunningham ??: > > Zhang Xiao Ping wrote: > >> I have updated http://cr.opensolaris.org/~yuntian/conflict/ >> >> 3. >> a) I change install-sfw as a ksh93 scripts: 1 #! /usr/bin/ksh93 and add 'set >> -o errexit'. >> >> b) Because other files all used '. ${SRC}/tools/install.subr', and it is the >> same as 'source ${SRC}/tools/install.subr', so i keep it. >> > Change this - its old files that use the '. ' (dot ), newer ones use > 'source ' > > >> 5. It is alright. OSR 10547 (for conflict) has been approved, it is BSD >> like license. >> >> Another aspect, please refer to: >> >> http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWdiffstat/copyright >> >> They are the same. >> > This is okay with me - in fact I prefer it. > > But you will find that all recent integrations have had the full licence > included in this file - if you think you can get it through the RTI > as-is then that's okay I think it okay :) The following two have been fixed. > > >> Others are fixed as your comments. >> > more comments ... > > 1. usr/src/cmd/conflict/Makefile.sfw > > Change line 41 so that 'configure' explicitly defines > > the '--prefix=', eg. change it to something like ... > > (cd $(VER); env - CC=$(CC) ./configure $(CONFIGURE_OPTIONS)) > > where CONFIGURE_OPTIONS is defined in Makefile.master > > 2. usr/src/cmd/conflict/install-sfw > > See above > > Every thing else looks okay to me; but you need to get someone else to > review it also. > > Paul > > >> ? 12/12/08 18:30, Paul Cunningham: >> >>> >>> See below for my comments ... >>> >>> >>> Paul >>> >>> >>> >>> Zhang Xiao Ping wrote: >>> >>>> I am porting 'conflict' to opensolaris, which is a tool display >>>> conflicting files in $PATH. >>>> >>>> >>>> Please kindly help review it. >>>> >>>> http://cr.opensolaris.org/~yuntian/conflict/ >>>> >>>> >>>> >>> === Start of Comments === >>> >>> >>> 1. usr/src/cmd/conflict/METADATA >>> >>> Adding missing fields, see .. >>> >>> http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines >>> >>> >>> 2. usr/src/cmd/conflict/Makefile.sfw >>> >>> Change 'env ' to 'env - ' >>> >>> Add 'env - ' in front of the 'make', etc., >>> >>> so it doesn't pick-up any random env variable. >>> >>> >>> 3. usr/src/cmd/conflict/install-sfw >>> >>> Roland Mainz wrote: >>> >>> > use /usr/bin/ksh93 or /usr/bin/bash for install-sfw* and >>> >>> > 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) >>> >>> >>> Maybe add 'exit 0' at the end >>> >>> >>> 4. usr/src/pkgdefs/SUNWconflict/Makefile >>> >>> You have your own 'depend file so remove the line .. >>> >>> 29 DATAFILES= depend >>> >>> >>> 5. usr/src/pkgdefs/SUNWconflict/copyright >>> >>> I'm no expert but is the content of this okay? Check >>> >>> within Sun. >>> >>> >>> 6. usr/src/pkgdefs/SUNWconflict/depend >>> >>> Move the 'Copyright' lines to after the >>> >>> 'CDDL HEADER END' header >>> >>> >>> This looks like the default 'depend' file; have >>> >>> you checked you have no other dependencies with the >>> >>> dependency checker script? If there are no others then >>> >>> remove this file and leave the 'DATAFILES= depend' in >>> >>> SUNWconflict/Makefile so it used the default file. >>> >>> >>> 7. usr/src/pkgdefs/SUNWconflict/pkginfo.tmpl >>> >>> Maybe change the DESC= line to something like ... >>> >>> DESC="conflict - displays conflicting filenames in your execution path >>> (6.0)" >>> >>> >>> 8. usr/src/pkgdefs/SUNWconflict/prototype_sparc >>> >>> >>> Cosmetic: Comment line ... >>> >>> 45 # List files which are I386 specific here >>> >>> "I386 specific" ??? >>> >>> >>> === End of Comments ===== >>> >>> >>> >> >
