Hi Paul, I have updated the webrev http://cr.opensolaris.org/~pd155743/rdiff-backup/ with the changes suggested. Please let me know if any changes required.
thanks, Pradhap.D Paul Cunningham wrote: > usr/src/cmd/Makefile - what's the 't' on the 'rpm2cpio' line ?? > > usr/src/cmd/rdiff-backup/METADATA - missing fields, see > http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines > > usr/src/cmd/rdiff-backup/Makefile.sfw - > use .. > 36 --librsync-dir=$(ROOT)$(CFGPREFIX) > > use .. > 33 cd $(VER); env - \ > (see http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines) > > use $(GTAR) rather than tar (line 39) > > usr/src/cmd/rdiff-backup/install-sfw > Apply these ... > Roland Mainz wrote: > > use /usr/bin/ksh93 or /usr/bin/bash for install-sfw* and add a > > $ set -o errexit # at the beginning and replace ". > > ${SRC}/tools/install.subr" with "source ${SRC}/tools/install.subr" > > (the idea is to catch failures in the script and abort it at > > that point, right now the script will just continue) > > usr/src/pkgdefs/SUNWrdiff-backup/pkginfo.tmpl - > change so that DESC= is more descriptive than NAME=, eg. > NAME="rdiff-backup is a backup utility" > DESC="rdiff-backup is a local/remote mirror and incremental backup > utility (1.2.1)" > > Paul > > > Pradhap Devarajan wrote: > >> Hi all, >> I have bumped up the version of rdiff-backup to 1.2.1. Can you please >> review the changes. Webrev is located at >> http://cr.opensolaris.org/~pd155743/rdiff-backup/ >> >> thanks >> Pradhap.D >> >> Pradhap Devarajan wrote: >> >>> Hi Paul, >>> Thanks for your comments. I have inlined my answers >>> >>> >>> >>>> === Start of Comments === >>>> >>>> 1. usr/src/cmd/rdiff-backup/METADATA >>>> I think most people use 'URL:' instead of >>>> 'COMMUNITY:', but check? >>>> >>>> >>>> >>> I will stick with URL . >>> >>> >>>> 2. usr/src/cmd/rdiff-backup/Makefile.sfw >>>> Do you need the 'real-all:' rule? >>>> >>>> >>>> >>> removed >>> >>> >>>> 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. >>>> >>>> >>>> >>> done >>> >>> >>>> 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? >>>> >>>> >>> yes >>> >>> >>>> 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' ? >>>> >>>> >>>> >>> done >>> >>> >>>> 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? >>>> >>>> >>>> >>> done >>> >>> >>>> Maybe the doc/rdiff-backup-1.0.5 directory should be >>>> doc/rdiff-backup ? >>>> >>>> >>> Most of the directory has version number under /usr/share/doc >>> >>> >>>> 7. Everything else looks okay to me. >>>> >>>> === End of Comments ===== >>>> >>>> >>> Please let me know if any comments. >>> >>> > >
