David, See below for my comments ...
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/"; 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
