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
