Sriram,

I took a *quick* looks at this, below are a few comments ...
Note, I haven't reviewed it properly though.

Paul

Sriram Natarajan wrote:
>  Please find the webrev at 
> http://cr.opensolaris.org/~sn123202/php528.feb9/webrev . I would very 
> much appreciate your insights
> 
> This webrev addresses the following bugs:
> 
> 6641450 [nvb79] php cgi tests are failing when using delivered php.ini : 
> php-cgi binary build issue
> 6685623 [nvb86] dtrace integration for php generates core dump for zend 
> tests
> 6768395 lighttpd fails to start due to the wrong php paths
> 6780290 php function getmyuid shows incorrect uid
> 6781705 integrate php 5.2.8 within opensolaris 2009.04
> 6793666 upgrade idn pecl extension to 0.2.0
> 6793668 uprade xdebug pecl extension to 2.0.4

=== Start of Comments ===

1. usr/src/cmd/php5/METADATA
    Add the missing fields, eg. NAME:, see
    http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines

2. usr/src/cmd/php5/Makefile.sfw
    Extract the following info from the METADATA file ..
      27 PHP_VER=5.2.8
      28 PHP_DIR=php-$(PHP_VER)
    see recent integration for examples.

3. usr/src/pkgdefs/SUNWphp52d/prototype_com
      & usr/src/pkgdefs/SUNWphp52u/prototype_com
    You may want to install any new files without the
    write permission bit set

4. usr/src/pkgdefs/SUNWphp52d/prototype_i386
      & usr/src/pkgdefs/SUNWphp52d/prototype_sparc
      & usr/src/pkgdefs/SUNWphp52r-mysql/prototype_com
      & usr/src/pkgdefs/SUNWphp52r-mysql/prototype_i386
      & usr/src/pkgdefs/SUNWphp52r-mysql/prototype_sparc
      & usr/src/pkgdefs/SUNWphp52r-pear/pkginfo.tmpl
      & usr/src/pkgdefs/SUNWphp52r-pear/prototype_com
      & usr/src/pkgdefs/SUNWphp52r-pear/prototype_i386
      & usr/src/pkgdefs/SUNWphp52r-pear/prototype_sparc
      & usr/src/pkgdefs/SUNWphp52r-pgsql/prototype_com
      & usr/src/pkgdefs/SUNWphp52r-pgsql/prototype_i386
      & usr/src/pkgdefs/SUNWphp52r-pgsql/prototype_sparc
      & usr/src/pkgdefs/SUNWphp52r/prototype_com
      & usr/src/pkgdefs/SUNWphp52r/prototype_i386
      & usr/src/pkgdefs/SUNWphp52r/prototype_sparc
      & usr/src/pkgdefs/SUNWphp52u-mysql/prototype_com
      & usr/src/pkgdefs/SUNWphp52u-mysql/prototype_i386
      & usr/src/pkgdefs/SUNWphp52u-mysql/prototype_sparc
      & usr/src/pkgdefs/SUNWphp52u-pear/pkginfo.tmpl
      & usr/src/pkgdefs/SUNWphp52u-pear/prototype_com
      & usr/src/pkgdefs/SUNWphp52u-pear/prototype_i386
      & usr/src/pkgdefs/SUNWphp52u-pear/prototype_sparc
      & usr/src/pkgdefs/SUNWphp52u-pgsql/prototype_com
      & usr/src/pkgdefs/SUNWphp52u-pgsql/prototype_i386
      & usr/src/pkgdefs/SUNWphp52u-pgsql/prototype_sparc
      & usr/src/pkgdefs/SUNWphp52u/prototype_i386
      & usr/src/pkgdefs/SUNWphp52u/prototype_sparc
    Why have you updated these files - I don't think you
    have changed anything other than the copyright
    year - remove the changes


    This makes it look as though you have changed a *lot* of
    files when really you haven't - so will save you time
    and not put of reviewers (theres too much of this stuff!)


5. usr/src/pkgdefs/SUNWphp52r-mysql/depend
    Remove the version on the line ...
     52 P SUNWphp52r    PHP Server Version 5.2.8 (Root)
    that way you will not have to update the file for
    every version update

    and in ...
    usr/src/pkgdefs/SUNWphp52r-pear/depend
    usr/src/pkgdefs/SUNWphp52r-pgsql/depend
    usr/src/pkgdefs/SUNWphp52u-mysql/depend
    usr/src/pkgdefs/SUNWphp52u-pear/depend
    usr/src/pkgdefs/SUNWphp52u-pgsql/depend
    usr/src/pkgdefs/SUNWphp52u/depend

6. usr/src/pkgdefs/SUNWphp52r-mysql/pkginfo.tmpl
    I don't think you need the version on the NAME= line
      NAME="PHP 5.2.8 module for MySQL (Root)"

    and in ...
    usr/src/pkgdefs/SUNWphp52r-pgsql/pkginfo.tmpl
    usr/src/pkgdefs/SUNWphp52r/pkginfo.tmpl
    usr/src/pkgdefs/SUNWphp52u-mysql/pkginfo.tmpl
    usr/src/pkgdefs/SUNWphp52u-pgsql/pkginfo.tmpl
    usr/src/pkgdefs/SUNWphp52u/pkginfo.tmpl

=== End of Comments =====

-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Business Unit

Reply via email to