Bugzilla will be upgraded to version 5.0 on a still to be determined date in the near future. The original upgrade date has been delayed.
Bug 239906 - Review Request: php-pear-Net-Ping - Execute ping
Review Request: php-pear-Net-Ping - Execute ping
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
Depends On:
  Show dependency treegraph
Reported: 2007-05-12 04:06 EDT by Remi Collet
Modified: 2007-11-30 17:12 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2007-06-02 03:51:19 EDT
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)

  None (edit)
Description Remi Collet 2007-05-12 04:06:12 EDT
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 19:27:53 EDT
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 06:59:35 EDT
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 16:22:32 EDT
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 01:48:13 EDT
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 13:02:58 EDT
Package Change Request
Package Name: php-pear-Net-Ping
New Branches: F-7

Comment 6 Jason Tibbitts 2007-06-01 16:21:09 EDT
CVS done.

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