This looks okay to me now Paul Zhang Xiao Ping wrote:
> > 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 ===== >>>> >>> >> > -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit General Dynamics Itronix Europe Ltd.
