Spoorthy H.S wrote:
>
> I have done the changes. Please review the same at
> http://cr.opensolaris.org/~spoorthy/perl-crypt-cbc4/
1. usr/src/Targetdirs
Doesn't look as though you have changed this so remove it
>> 5. usr/src/lib/perl_crypt_cbc/Makefile.sfw
>> Why are you not using lines 28 & 29 ? ...
>> 28 #VER = $(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh)
>> 29 #TARBALL = $(VER).tar.gz
>> 30 VER = Crypt-CBC-2.30
>> 31 TARBALL = Crypt-CBC-2.30.tar.gz
>>
> Done
Why are you still not using the info extracted from the METADATA (you
have it hardcoded)?
Also in the METADATA should NAME: be Crypt-CBC so you can extract it
into the VER ?
>> 6. usr/src/pkgdefs/SUNWperl-crypt-cbc/depend
>> Your Makefiles says you are using the default 'depend' file
>> so you can delete this file. But are you sure you have no
>> other dependencies that should be included?
>>
> I need this file. This package is dependent on the other package called
> SUNWperl-crypt-des. I am working on porting this package in parallel.
You should therefore delete the line ...
29 DATAFILES=depend
from your SUNWperl-crypt-cbc/Makefile
And as you are keeping your 'depend' you should move the copyright lines
down to after the 'CDDL HEADER END'.
The SCCS ident line looks wrong also.
>> 7. usr/src/pkgdefs/SUNWperl-crypt-cbc/copyright
>> It might be better to wrap the 'long' lines in this file
I still think you should wrap the lines in here instead of having the
very long lines!
>> 8. usr/src/pkgdefs/SUNWperl-crypt-cbc/pkginfo
>> Delete this file (from checked in) as its autogenerated
>> from pkginfo.tmpl
>>
> Done
It is still in the webrev!
>> 9. usr/src/pkgdefs/SUNWperl-crypt-cbc/pkginfo.tmpl
>> Add pkg version at the end of the DESC line, eg ..
>> DESC="................... (2.30)"
>>
> Added version of the package
It's more commonly put in brackets as (x.x).
>> The SCCS ident line doesn't look right (and check it in other
>> files)
These still all look wrong to me !
>> 10. usr/src/pkgdefs/SUNWperl-crypt-cbc/prototype_sparc
>> Line 46 says 'i386' ???
>>
> Changed it
It still looks wrong in the webrev!
Paul
--
Paul Cunningham
Software Engineer
Tel:
