Hi Paul,

Please review it now. The changes are at 
http://cr.opensolaris.org/~spoorthy/cryptws/

Thanks
Spoorthy

Paul Cunningham wrote:
>
>
> 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
>

Reply via email to