Hi Paul, I am extremely sorry, I had skipped the review for this package. Updated most of the things in the packages. Pl. see my comments INLINE.
Webrev will be followed in a short while. Thanks, ~Vivek R. Titarmare -----Original Message----- From: Paul Cunningham [mailto:[email protected]] Sent: Wednesday, May 06, 2009 1:58 PM To: Vivek Titarmare Cc: sfwnv-discuss at opensolaris.org Subject: Re: [sfwnv-discuss] Request code review for "tipc" Comments below .. Paul Vivek Titarmare wrote: > I have posted a webrev for package "tipc" which I am porting to Neveda > and would like to request a code review. Please see below link > > http://cr.opensolaris.org/~vivekrt/6833850-tipc/ 1. usr/src/lib/tipc/Makefile.sfw Change 'env ' to 'env - ' throughout Do you need ? .. 81 (cd $(CODE_PATH)/$(VER)/usr/src ; \ 82 env "SRC=$(CODE_PATH)/$(VER)/usr/src" \ 83 "MAKEFLAGS=e" \ 84 make clean) when you then do ... 85 rm -rf $(VER) Apply the patches in the '$(VER):' rule rather than in the 'all:' rule I assume the .. 72 make release builds and installs the files; only so the build in the 'all:' rule, then do the install in the 'install:' rules [VIVEK]: Lines removed and changes related to patches and install are done. 2. Tops of files (various) Cosmetic: Change so that they conform to those in ... "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; ie, mainly add missing line space after CDDL HEADER END [VIVEK]: changed all the headers. 3. usr/src/lib/tipc/install-sfw Delete line as its not used .. 30 TOP=`pwd` Replace with a line-space Add a 'set -o errexit' as in .. "http://src.opensolaris.org/source/xref/sfw/usr/src/cmd/meld/install-sfw"; [VIVEK]: line deleted and added set -o errexit 4. patches Note, I haven't looked at these [VIVEK] ok. 5. usr/src/pkgdefs/Makefile & usr/src/Targetdirs These need resyncing with gate so it doesn't look as though you are trying to change other stuff. [VIVEK]: resynced. 6. usr/src/pkgdefs/SUNWtipc/Makefile Copyright year is wrong; also ensure you check all the other files [VIVEK]: changes all headers, this is also changed. 7. usr/src/pkgdefs/SUNWtipc/copyright The METADATA file say ... LICENSE: BSD GPL the GPL licence is not in here! [VIVEK]: it should only be BSD. 8. usr/src/lib/tipc/METADATA Line ... 9 BUGTRAQ: /solaris/utility/tipc should it have the first '/' ? [VIVEK]: changed. 9. usr/src/pkgdefs/SUNWtipc/depend Copyright lines are in the wrong place and the year is wrong. [VIVEK]: changed 10. usr/src/pkgdefs/SUNWtipc/prototype_com Line is wrong ... 32 ARCH=i386 and there is some other stuff missing I think (in fact this looks like an after 'pkmake' state of file!!) Add pkg version at end of DESC= line ... DESC="............ (n.x.x)" [VIVEK]: changed. 11. usr/src/pkgdefs/SUNWtipc/prototype_com Don't install file is /usr with the write permissions set! [VIVEK]: changed 12. usr/src/pkgdefs/SUNWtipc/prototype_i386 & New Patch Raw usr/src/pkgdefs/SUNWtipc/prototype_sparc Move the list of files to the end of the file as in prototype_com [VIVEK]: changed PLEASE DO A SELF REVIEW OF YOUR STUFF BEFORE SENDING OUT THESE WEBREVS so I and other do not waste their time picking I silly errors !!! END -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
