See comments below from quick skip through ...

Paul

Spoorthy H.S wrote:

> 
> Please find the new webrev
> http://cr.opensolaris.org/~spoorthy/perl-crypt-cbc1/

>>>
>>> I am porting the package Crypt-CBC. Please review the code and reply 
>>> me for the changes. Webrev is at 
>>> http://cr.opensolaris.org/~spoorthy/crypt-cbc/
>>>     

1. usr/src/Makefile
    Doesn't look as though you have changed this so remove it

2. usr/src/Makefile
    Move perl_crypt_cbc so that they are in alphabetically order

3. usr/src/lib/perl_crypt_cbc/METADATA
    Add correct entry for BUGTRAQ

4. usr/src/lib/perl_crypt_cbc/Makefile.sfw
     + usr/src/lib/perl_crypt_cbc/install-sfw
     + any other files
    Remove extra space chars in the CDDL header lines, ie. ..
      #  .....
    becomes ..
      # .....

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

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?

7. usr/src/pkgdefs/SUNWperl-crypt-cbc/copyright
    It might be better to wrap the 'long' lines in this file

8. usr/src/pkgdefs/SUNWperl-crypt-cbc/pkginfo
    Delete this file (from checked in) as its autogenerated
    from pkginfo.tmpl

9. usr/src/pkgdefs/SUNWperl-crypt-cbc/pkginfo.tmpl
    Add pkg version at the end of the DESC line, eg ..
      DESC="................... (2.30)"

    The SCCS ident line doesn't look right (and check it in other
    files)

10. usr/src/pkgdefs/SUNWperl-crypt-cbc/prototype_sparc
     Line 46 says 'i386' ???

-- 
Paul Cunningham
Software Engineer

Reply via email to