Cunningham, Paul - UK wrote: Hi,
Here's another go at code review: http://cr.opensolaris.org/~cn162287/logrotate/webrev Changes has been made as per the last 2 reviews. There are now 2 packages: SUNWlogrotater and SUNWlogrotateu Thanks. colin >> what is a root package? >> > > I Believe (someone correct me if I am wrong please) - It's just another > package that contains all the stuff for the package that must be install > on the local machine, eg. the /etc stuff, whereas the stuff in the base > package can be installed & mounted from a remote server. So you just > need to create a second package with the /etc stuff in it - these root > packages normally have an 'r' at the end of their name and '(root)' on > the pkginfo DESC line - take a look in the gate at other root packages. > > > See below for other comments ... > > Paul > > >> -----Original Message----- >> From: sfwnv-discuss-bounces at opensolaris.org >> [mailto:sfwnv-discuss-bounces at opensolaris.org] On Behalf Of Colin Ngam >> Sent: 25 June 2008 22:20 >> To: Cunningham, Paul - UK >> Cc: sfwnv-discuss at opensolaris.org; solaris-qe-pkgproj at Sun.COM >> Subject: Re: [sfwnv-discuss] Request for code review - >> logrotate utility >> >> Cunningham, Paul - UK wrote: >> >>> Colin, >>> >>> See below for my comments from my quick skip through ... >>> >>> Paul >>> >>> >>> >>>> -----Original Message----- >>>> From: sfwnv-discuss-bounces at opensolaris.org >>>> [mailto:sfwnv-discuss-bounces at opensolaris.org] On Behalf Of Colin >>>> Ngam >>>> Sent: 23 June 2008 14:45 >>>> To: sfwnv-discuss at opensolaris.org; solaris-qe-pkgproj at Sun.COM >>>> Cc: Colin Ngam >>>> Subject: [sfwnv-discuss] Request for code review - >>>> >> logrotate utility >> >>>> Hi, >>>> >>>> This is a request for code review. Please see >>>> http://cr.opensolaris.org/~cn162287/logrotate/webrev >>>> >>>> And provide any comments or changes I need to make. >>>> >>>> >>> === Start of Comments === >>> >>> 1. usr/src/cmd/Makefile >>> & usr/src/pkgdefs/Makefile >>> This needs resyncing with gate otherwise it looks as >>> though you are trying to delete stuff. >>> >>> >> okay >> > > >>> 2. usr/src/pkgdefs/SUNWlogrotate/prototype_com >>> Shouldn't the 'etc/...' stuff be in a separate >>> root package? >>> >>> >> Can someone help me out here? I am installing a sample >> configuration file into /etc/logrotate.conf .. what is a >> root package? >> >> PKG="SUNWlogrotate" >> NAME="Logrotate - rotates and compresses log file" >> ARCH="ISA" >> VERSION="SFWVERS,REV=0.0.0" >> SUNW_PRODNAME="SunOS" >> SUNW_PRODVERS="RELEASE/VERSION" >> SUNW_PKGTYPE="usr" >> MAXINST="1000" >> CATEGORY="system" >> DESC="Logrotate - rotates and compresses log file (3.7.1)" >> VENDOR="Sun Microsystems, Inc." >> HOTLINE="Please contact your local service provider" >> EMAIL="" >> CLASSES="none" >> BASEDIR=/ >> SUNW_PKGVERS="1.0" >> SUNW_PKG_ALLZONES="false" >> SUNW_PKG_HOLLOW="false" >> SUNW_PKG_THISZONE="false" >> >> The prototype_com file looks like: >> >> d none usr 0755 root sys >> d none usr/sbin 0755 root bin >> f none usr/sbin/logrotate 0555 root bin >> d none usr/share 0755 root sys >> d none usr/share/man 0755 root bin >> d none usr/share/man/man1 0755 root bin >> f none usr/share/man/man1/logrotate.1 0444 root bin >> >> >>> 3. usr/src/pkgdefs/SUNWlogrotate/depend >>> I can't see this file link - permissions wrong? >>> >>> >> Corrected >> > > I still can't see this - have you update your webrev? > > >>> 4. usr/src/cmd/logrotate/METADATA >>> I don't think its normal to have the sccs ident >>> stuff in a METADATA file. I don't know if it >>> matters though either. >>> >>> >> Tools complain .. so I added it to shut it up. >> > > I wonder why no one else has not had in complain? - unless they ignore > it. > > >>> 5. usr/src/cmd/logrotate/Makefile.sfw >>> Does this source package have a 'configure' file? if not >>> you may want to add comment to say so and that the 'touch' >>> is just there to aid make >>> >>> >> No configure file. >> > > I still think then that you should add a comment into the file to say > what the '$(VER)/configure' is used for if there is not one in the src > pkg. > > >>> 6. usr/src/cmd/logrotate/install-sfw >>> The 'INFODIR=' line is not used so delete it >>> >>> >> Okay. >> > > >>> === End of Comments ===== >>>
