Sriram,

This mainly looks good to me also now, see below for a couple of minor 
comments ...

Pail

Sriram Natarajan wrote:
> 
> Please find the updated webrev with the changes that you requested included
> http://cr.opensolaris.org/~sn123202/php529.feb26/

>>
>> Sriram Natarajan wrote:
>>> We would like to integrate MCrypt extension and implement some of the 
>>> PHP enhancements mentioned within LSARC/2009/124 ARC case like adding 
>>> hook up for sun web server 7 and ability to provide statistics for 
>>> APC and Memcache within  build 110. Can you please review my webrev 
>>> and provide your valuable feedback.
>>> http://cr.opensolaris.org/~sn123202/php529.feb26/
>>>
>>> Though, this web rev does not contain changes pertaining to 5.2.9 
>>> integration, I hope to commit 5.2.9 as a separate putback. I am still 
>>> waiting for that RTI to be approved.
>>>
>>> Note: We are currently unable to complete adding DTrace probes within 
>>> PHP engine for build 110 and I guess, hopefully we will be allowed to 
>>> integrate within build 111.

1. usr/src/cmd/php5/METADATA
    Make the NAME: line more descriptive

    Change COMMUNITY: to URL: (to be consistent across
    pkgs), see ...
    http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines

2. CDDL HEADER and tops of file (various files)
    Cosmetic: You could update these so that they conform
    to those in ...
"http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/";
See http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines
    probably mainly the missing line space after CDDL header

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

Reply via email to