Bug 239906 - Review Request: php-pear-Net-Ping - Execute ping
Summary: Review Request: php-pear-Net-Ping - Execute ping
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
(Show other bugs)
Version: rawhide
Hardware: All Linux
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
Depends On:
TreeView+ depends on / blocked
Reported: 2007-05-12 08:06 UTC by Remi Collet
Modified: 2007-11-30 22:12 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2007-06-02 07:51:19 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora-review+
tibbs: fedora-cvs+

Attachments (Terms of Use)

Description Remi Collet 2007-05-12 08:06:12 UTC
Spec URL: http://remi.collet.free.fr/rpms/extras/php-pear-Net-Ping.spec
SRPM URL: http://remi.collet.free.fr/rpms/extras/php-pear-Net-Ping-2.4.1-1.fc8.src.rpm
Mock Log: http://remi.collet.free.fr/rpms/extras/php-pear-Net-Ping-build.log
OS independet wrapper class for executing ping calls


This extension is required by "oreon"

Comment 1 Jason Tibbitts 2007-05-15 23:27:53 UTC
I suggest changing %description to:
  OS independent wrapper class for executing ping calls.
to correct a misspelling and add a period.

I think this package should have a depdendency on iputils (for /bin/ping). 
Otherwise I don't see how it could perform its function.

rpmlint complains:
  W: php-pear-Net-Ping incoherent-version-in-changelog 1.4.1-1 2.4.1-1.fc7
The package version is 2.4.1 but the (only) changelog entry refers to 1.4.1. 
Probably just a typo.

There is a basic test included which can easily be run at build time with:
   cd tests
   php test_result_parsers.php

* source files match upstream:
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
X rpmlint has a valid complaint.
? final provides and requires:
   php-pear(Net_Ping) = 2.4.1
   php-pear-Net-Ping = 2.4.1-1.fc7

X %check is not present, but there's a runnable test suite.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

Comment 2 Remi Collet 2007-05-16 10:59:35 UTC
Thanks for the review.

Spec URL: http://remi.collet.free.fr/rpms/extras/php-pear-Net-Ping.spec

* Wed May 16 2007 Remi Collet <Fedora@FamilleCollet.com> 2.4.1-2
- From review, correct typo.
- add Requires: /bin/ping
- add simple test in %%check 

I requires /bin/ping rather than iputils after asking #fedora-devel. I don't
really prefer one solution to another. It will work even if ping gets moved to
another package.

When packaging pear stuff, most often, tests must be run after installing the
package (doesn't run in Buildroot, even with R changed to BR).

Running tests after installation allow to be more complete (need configuration
change, databse...), ex running

But, of course, i agree to include test when exists and works.

Comment 3 Jason Tibbitts 2007-05-16 20:22:32 UTC
Well, file dependencies in /bin are fine according to the guidelines.  (There
are several directories for which they are OK, but outside of those yum has to
fetch additional dependency information which makes things slow.)

I know that most PEAR modules don't have tests which are runnable at build time,
but I happened to notice that this one could be run which is why I pointed it out.

Everything looks good to me now.


Comment 4 Remi Collet 2007-05-17 05:48:13 UTC
New Package CVS Request
Package Name: php-pear-Net-Ping
Short Description: Execute ping
Owners: Fedora@FamilleCollet.com
Branches: devel FC-5 FC-6 EL-5 (FC-7 ???)

Comment 5 Remi Collet 2007-06-01 17:02:58 UTC
Package Change Request
Package Name: php-pear-Net-Ping
New Branches: F-7

Comment 6 Jason Tibbitts 2007-06-01 20:21:09 UTC
CVS done.

Note You need to log in before you can comment on or make changes to this bug.