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=551723 --- Comment #2 from Christof Damian <christof@xxxxxxxxxx> 2010-01-26 15:06:42 EST --- (In reply to comment #1) > First notes (before review): > > - According to package.xml and "pci" command result, it requires php-xml (dom + > simpleXML) OK > - directory /usr/share/pear/PHP is not owned I was not sure. I thought these common directories shouldn't be owned. > - should use %{channel} in %postun to have consistent use of macros OK > Optional deps : it's your choice to include this, or not > > PHP_Depend-0.9.9 can optionally use package "pecl/imagick" (version >= 2.2.0b2) > > As this package is available, you could add > Requires: php-pecl(imagick). > A comment about this as "optionnal" seems usefull. > If you don't want to add it, please add a note in the package description. I have added it, it makes sense as it is available in Fedora anyway. > Why do you remove test from RPM ? > > Adding %{pear_testdir}/%{pear_name} seems usefull (nothing on the Guidelines > about this, but most of pear packages which provides tests keep it bundled) I had a look at other packages and some seem to remove them and some keep them. I planned to add them again anyway after seeing some other packages doing it. > Do you know how to run the test suite providesd? > I've try (but most fails) > $ cd /usr/share/pear/test/PHP_Depend/tests/PHP/Depend/ > $ phpunit AllTests > (it's also a good idea, if you couldn't run it during build to add a comment > about how to run it in your spec) To run them you have to create the /usr/share/pear/test/PHP_Depend/tests/PHP/Depend/_run directory and I think the tree has to be writeable. Once you do that you get: OK, but incomplete or skipped tests! Tests: 1145, Assertions: 3601, Skipped: 3. If it isn't writeable you get: FAILURES! Tests: 1145, Assertions: 3478, Failures: 6, Errors: 32, Skipped: 1. I don't think that they are made to be run after install. I made a new version: Spec URL: http://rpms.damian.net/SPECS/php-pdepend-PHP-Depend.spec SRPM URL: http://rpms.damian.net/SRPMS/php-pdepend-PHP-Depend-0.9.9-2.fc12.src.rpm -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review