[Bug 488185] Review Request: php-pecl-selinux - SELinux binding for PHP scripts

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=488185


Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mtasaka at ioa.s.u-tokyo.ac.jp
             Blocks|177841(FE-NEEDSPONSOR)      |




--- Comment #2 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp>  2009-03-04 14:54:09 EDT ---
(Removing NEEDSPONSOR)

Well, I am familiar with neither php nor selinux, however
some comments

* rpm name
  - Please make Name consistent first.
    - I guess this rpm should be named as "php-pecl-selinx" as
      the spec file suggests.
    - However currently Name uses "php-selinux".

* Versioning
  - If this is the pre-release of formal 0.1.2 release,
    please follow
   
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages
    (Anyway using "devel" as Release seems strange)

* %__pecl
  - To build this package on koji,
------------------------------------------------------
%{!?__pecl:     %{expand: %%global __pecl     %{_bindir}/pecl}}
------------------------------------------------------
    cannot be removed because
    - When buildroot is initialized, no PHP related rpms
      are installed yet, so %__pecl is not defined at this stage.
    - Then mock tries "rpm -bs --nodeps foo.spec".
      Then rpm complains like
------------------------------------------------------
error: line 14: Dependency tokens must begin with alpha-numeric, '_' or '/':
Requires(post): %{__pecl}
------------------------------------------------------

* %if %{?php_zend_api}0
  - Well, actually Fedora guideline actually suggests so, however
    generally this should be "if 0%{?php_zend_api}" (no deference
    for this case, however this is usual usage)

* BR (BuildRequires)
  - Would you check if the following message in build.log ignored?
------------------------------------------------------
    81  checking for re2c... no
    82  configure: WARNING: You will need re2c 0.13.4 or later if you want to
regenerate PHP parsers.
------------------------------------------------------

* %post scriptlet
------------------------------------------------------
%post
%{pecl_install} %{pecl_xmldir}/%{name}.xml >/dev/null || :
%endif
------------------------------------------------------
  - However %{pecl_xmldir}/%{name}.xml does not seem to be
    installed.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.