Pradhap, I did a quick skip through, see below for my comments
Paul On Thu, 2008-07-17 at 13:52, Pradhap Devarajan wrote: > Hi all, > > Please review the code for rdiff-backup integration into SFW consolidation > > webrev is located at: > http://cr.opensolaris.org/~pd155743/rdiff-backup/ > === Start of Comments === 1. usr/src/cmd/rdiff-backup/METADATA I think most people use 'URL:' instead of 'COMMUNITY:', but check? 2. usr/src/cmd/rdiff-backup/Makefile.sfw Do you need the 'real-all:' rule? 3. usr/src/cmd/rdiff-backup/install-sfw The following are not used BINDIR=, LIBDIR=, INCDIR= & MAN3DIR so delete. The director created by 'mkdir -p ${DOCDIR}' should be in the Targetdirs file so you don't need these lines, just add it to Targetdirs. 4. usr/src/pkgdefs/SUNWrdiff-backup/depend Move the 'Copyright' lines to after the 'CDDL HEADER END' header. Have you run the dependency checking script on your package? 5. usr/src/pkgdefs/SUNWrdiff-backup/pkginfo.tmpl The NAME= line is more descriptive than the DESC= line. Maybe swap them? Current DESC= line, 'is backup' maybe should be 'is a backup' ? 6. usr/src/pkgdefs/SUNWrdiff-backup/prototype_com Line .. '116 d none usr//share 0755 root bin' has double / Do you need to deliver the CHANGELOG file? Maybe the doc/rdiff-backup-1.0.5 directory should be doc/rdiff-backup ? 7. Everything else looks okay to me. === End of Comments ===== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Computer Products
