Thanks Paul for the review. I will update them and send for code review again. Regarding 10, there are no dependencies for SUNWconmanu except the root package. Also, How can I run the "package dependency checker" ? Is there any specific command for it ? If yes, give me the pointer.
Thanks Spoorthy ----- Original Message ----- From: Paul Cunningham <[email protected]> Date: Tuesday, March 24, 2009 7:41 pm Subject: Re: [sfwnv-discuss] Code review request for conman To: "Spoorthy H.S" <Spoorthy.Shankarmurthy at Sun.COM> Cc: SFW-NV Discuss <sfwnv-discuss at opensolaris.org>, Srirama Sharma <Srirama.Sharma at Sun.COM> > Spoorthy, > > See below for my comments ... > > Paul > > Spoorthy H.S wrote: > > > >New webrev is at > >http://cr.opensolaris.org/~spoorthy/conmanupd/ > > === Start of Comments === > > 1. usr/src/Targetdirs > Put your new bits in alphabetically ... > /etc/apache2/2.2 \ > /etc/apache2/2.2/conf.d \ > + /etc/logrotate.d \ > + /etc/default \ > /etc/lighttpd \ > /etc/lighttpd/1.4 \ > /etc/lighttpd/1.4/conf.d \ > /etc/openwsman \ > > 2. usr/src/cmd/conman/METADATA > Remove the last blank line > > Complete this line ... > 11 BUGTRAQ: > > 3. usr/src/cmd/conman/Makefile.sfw > On line 33 you change ... > --sysconfdir=/etc > to .. > --sysconfdir=$(CFGETC) > > Cosmetic: move the line-space up a line, eg. .. > 19 # CDDL HEADER END > 20 # > .. > 21 # > 23 # Copyright 2009 Sun Microsystems, Inc. All rights reserved. > > 4. usr/src/cmd/conman/install-sfw > Cosmetic: add extra line-space after line 45 > > On lines 74 to 83 you could use ${LIB} instead of > ${PREFIX}/lib > > 5. Various SUNWconmanr/* and SUNWconmanu/* files > Change the top of these files so they all conform to > that in ... > "http://src.opensolaris.org/source/xref/onnv/onnv- > gate/usr/src/prototypes/" > 6. usr/src/pkgdefs/SUNWconmanr/pkginfo.tmpl > As this is a root package you may want to remove the > pkg version number on the DESC= line (that way this > file should not need changing when the src pkg > is up versioned) > > 7. usr/src/pkgdefs/SUNWconmanr/prototype_com > Do any of the installed /etc files need preserving > over a SUNW pkg upgrade? If so you need to handle that. > > 8. pkgdefs/Makefile > & cmd/Makefile > The changes to these are not in the webrev > > 9. usr/src/pkgdefs/SUNWconmanu/copyright > Add src pkg owner copyright lines (after sun disclaimer) > extracted from the src files in the uncompressed tarball, > see example at ... > "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright"; > > 10. usr/src/pkgdefs/SUNWconmanu/depend > Are there any other dependencies? > > === End of Comments ===== > -- > -------------------------------------------------------------------- > -- > Paul Cunningham > Software Engineer > Tadpole Business Unit >
