[Bug 785606] Review Request: php-horde-Horde-Test - Horde testing base classes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Shawn Iwinski <shawn.iwinski@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fedora@xxxxxxxxxxxxxxxxx,
                   |                            |shawn.iwinski@xxxxxxxxx

--- Comment #1 from Shawn Iwinski <shawn.iwinski@xxxxxxxxx> ---
I am not an official package reviewer, but here are some comments I have for
this package:



*** It would help readability to group your Build* and Requires* statements
together instead of mixing them with each other and your Provides and Conflicts
statements



*** Your Provides statement should be
Provides:    php-pear(pear.horde.org/%{pear_name}) = %{version}
instead of
Provides:    php-pear(%{pear_name}) = %{version}



*** You should run phpci on your packages to make sure you have all requires:

$ phpci print --recursive --report=extension src/Horde_Test-1.3.0
22 / 22 [+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++>] 100.00%
BASE: /projects/fedora/REVIEWS/785606/src/Horde_Test-1.3.0
-------------------------------------------------------------------------------
PHP COMPAT INFO EXTENSION SUMMARY
-------------------------------------------------------------------------------
  EXTENSION                                        PECL   VERSION         COUNT
-------------------------------------------------------------------------------
  Core                                                    4.0.0              52
  PDO                                           1.0.4dev  5.1.0               1
  SPL                                                0.2  5.0.0               8
  dom                                           20031129  5.0.0               4
  json                                             1.2.1  5.2.0               1
  pcre                                                    4.0.0               4
  standard                                                4.0.0              63
-------------------------------------------------------------------------------
A TOTAL OF 7 EXTENSION(S) WERE FOUND
REQUIRED PHP 5.2.0 (MIN) 
-------------------------------------------------------------------------------
Time: 1 second, Memory: 9.25Mb
-------------------------------------------------------------------------------

According to phpci, you need to require "php-pdo" and "php-dom" (virtual
provide of php-xml) at a minimum.  File
Horde_Test-1.3.0/lib/Horde/Test/Functional.php uses the dom functions.  File
Horde_Test-1.3.0/lib/Horde/Test/Factory/Db.php uses the PDO functions.  While
package.xml lists these as optional, I believe this package would provide a
better end-user experience if you added the two requires:

Requires: php-pdo
Requires: php-dom

spl, json, and pcre are all virtual provides from php-common. For completeness
(and to prevent any future packaging issues due to PHP package changes) my
sponsor and I have agreed to require all virtual provides.  It is up to you,
but I would suggest adding the following as well:

Requires: php-spl
Requires: php-json
Requires: php-pcre



*** Do you plan on building for EPEL 5?
- If not, please remove "rm -rf $RPM_BUILD_ROOT" from the %install section (see
http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag).
- If you do, there are several staements that need to be added.



*** Up to you, but package.xml lists the following optional packages if you
choose to require:
- php-pear(pear.horde.org/Horde_Cli)
- php-pear(pear.horde.org/Horde_Log)
(if you do choose to require these packages, please update this ticket's
"Depends On")

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]