Grace, This looks good to me now
Paul Grace Tang wrote: > > Thanks a lot for your review. I've addressed your comments and updated > the webrev. > > http://cr.opensolaris.org/~grace/dwdiff/ > > Also, please see my response in lines. > > Paul Cunningham wrote: >> >> See below for my comments ... >> >> >> Grace Tang wrote: >>> >>> I am porting 'dwdiff', a diff program that operates at the word level, >>> to OpenSolaris. >>> >>> Can you please review my code? The webrev is at: >>> >>> http://cr.opensolaris.org/~grace/dwdiff/ >> >> === Start of Comments ==== >> >> 1. usr/src/cmd/dwdiff/METADATA >> Make the NAME: field more descriptive. > Accepted. >> >> 2. usr/src/cmd/dwdiff/Makefile.sfw >> Extract the version number on 'VER =' from the MEATADATA > Accepted. >> >> Add "env - " before 'configure' and 'make' so it does not >> pick up random environment vars. > Accepted. >> >> Explicitly define 'configure --prefix=... " using >> CONFIGURE_OPTIONS from Makefile.master, eg. something >> like .... >> CONFIGURE_OPTIONS += --without-unicode >> .... >> .... >> ./configure $(CONFIGURE_OPTIONS) > Accepted. >> >> Maybe explicitly define the compile to use, CC=... > Accepted. >> >> Maybe use $(CCSMAKE) instead of 'make' > Accepted. >> >> 3. CDDL HEADER and file tops (all files) >> Check that they conform to those in ... >> "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; >> > Updated CDDL HEADER and file tops in all related files. >> >> 4. usr/src/pkgdefs/SUNWdwdiff/copyright >> Do you need the Sun disclaimer? as in ... >> "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright"; >> >> > Yes, Sun disclaimer is needed. I added it at the beginning. >> >> 5. usr/src/pkgdefs/SUNWdwdiff/depend >> Have you checked the pkg dependencies using the >> dependency checker script? > Yes, I used depend.sh Chris wrote. >> >> SUNWlibmsr, is this a dependency of your pkg? > Yes, dwdiff links to libm.so which is in SUNWlibmsr. >> >> 6. usr/src/pkgdefs/SUNWdwdiff/pkginfo.tmpl >> Added version at the end of the DESC= line, eg. ... >> DESC="................. (1.5.2)" > Accepted. >> >> 7. usr/src/pkgdefs/SUNWdwdiff/prototype_i386 >> & usr/src/pkgdefs/SUNWdwdiff/prototype_sparc >> To be consistent with other pkgs add comment line .. >> # SUNWdwdiff >> as in ... >> "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/prototype_i386"; >> >> > Accepted. >> >> === End of comments ===== > -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
