David, Paul Cunningham wrote: > > See below for my comments ... I missed a bit, added below ...
Paul > David Zhang wrote: >> >> Thank you very much for your detailed comments, I have addressed most >> of them and post it on http://cr.opensolaris.org/~ydzhang/areca/. >> >> However, I have questions for 4.b using (Targetdirs) instead of 'mkdir >> -p'. Would you please send me an example? > >>> David Zhang wrote: >>>> Would you please kindly review those webrev for opensolaris package >>>> SUNWareca? >>>> http://cr.opensolaris.org/~ydzhang/areca/ > > > === Start of Comments === > > 1. usr/src/pkgdefs/Makefile > & usr/src/cmd/Makefile > These need to be resynced with the gate otherwise it looks > as though you are trying to change other stuff > > 2. Top of files (usr/src/cmd/areca/Makefile.sfw & maybe others) > Cosmetic: change so that they conform to the prototypes > in (mainly missing line-space) ... > "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; 2A. usr/src/cmd/areca/Makefile.sfw Add after line 27 ... TARBALL=$(VER).tar.gz then replace '$(VER).tar.gz' with '$(TARBALL)' throughout > 3. usr/src/cmd/areca/install-sfw > The following are not used so delete ... > 31 MANDIR=${PREFIX}/share/man/man1 > 32 DOCDIR=${ROOT}/usr/share/doc/areca > but before you do; should you have created, and packaged, > a man page for this? (MANDIR looks wrong anyway) > > Whats this line for .. > 39 #install .sh > > Do you need the lines 'i=""' ? if not remove them. > > 3A. usr/src/Targetdirs > I think you need to add the dir ... > usr/share/areca > used in install-sfw (line 30) into here. > And any others dirs that you install into that are not > in this file. > > 4. usr/src/pkgdefs/SUNWareca/Makefile > You have your own 'depend' file so delete line .. > 30 DATAFILES = depend > > 5. usr/src/pkgdefs/SUNWareca/copyright > Add source code owner copyright lines after the Sun > disclaimer, see example in SUNWmeld/copyright. These lines > are extracted from the files in the unpacked source tarball. > "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright"; > > > > 6. usr/src/pkgdefs/SUNWareca/prototype_com > Move the creation of the symbolic link ... > 46 s none usr/bin/areca=../share/areca/areca.sh > to after the areca.sh has been installed ... > 49 f none usr/share/areca/areca.sh 0555 root bin > > 7. usr/src/pkgdefs/SUNWareca/depend > You deliver some 'jar' files in prototype_com so shouldn't > there be a dependency on the java runtime package? > > 8. usr/src/pkgdefs/SUNWareca/prototype_sparc > & usr/src/pkgdefs/SUNWareca/prototype_i386 > Cosmetic: Add SUNW package name in comment near the bottom as > in ... > "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/prototype_i386"; > > > so that it is consistent. > > === End of Comments ===== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
