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.
>>
--
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Business Unit
