Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 215200 (pear-Svcs-Weather) - Review Request: php-pear-Services-Weather - This class acts as an interface to various online weather-services
Summary: Review Request: php-pear-Services-Weather - This class acts as an interface t...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: pear-Svcs-Weather
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Stone
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: php-pear-SOAP
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-11-12 10:31 UTC by Remi Collet
Modified: 2010-05-13 22:39 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-11-22 06:19:14 UTC
Type: ---
Embargoed:
dennis: fedora-cvs+


Attachments (Terms of Use)

Description Remi Collet 2006-11-12 10:31:34 UTC
Spec URL: http://remi.collet.free.fr/rpms/extras/php-pear-Services-Weather.spec
SRPM URL: http://remi.collet.free.fr/rpms/extras/php-pear-Services-Weather-1.4.0-1.fc7.src.rpm
Mock Log: http://remi.collet.free.fr/rpms/extras/php-pear-Services-Weather-build.log
Description: 
Services_Weather searches for given locations and retrieves current
weather data and, dependent on the used service, also forecasts.
Up to now, GlobalWeather from CapeScience, Weather XML from EJSE (US
only), a XOAP service from Weather.com and METAR/TAF from NOAA are
supported. Further services will get included, if they become available,
have a usable API and are properly documented.

Comment 1 Christopher Stone 2006-11-18 17:15:35 UTC
Hi Remi, quick comment before I start the formal review. I looked over your spec
file and I wonder why you choose to use # as your delimiter character in your
sed command?  # is used as a comment in a spec file and this looks very
confusing to a reader of the spec file, please change # to @ instead.

Also you use %{__sed} however you do not use macros for any other of your system
commands, you should be consistent and just use "sed", I think it came up during
the discussion of creating the template that macro usage for system commands was
only needed in %post(pre)/%post(pre)un sections.

Therefore, I would change the sed line to read:
sed -i -e s@/usr/local/bin/php@%{_bindir}/php@
$RPM_BUILD_ROOT%{pear_datadir}/%{pear_name}/buildMetarDB.php


Comment 2 Christopher Stone 2006-11-18 17:39:51 UTC
==== REVIEW CHECKLIST ====
- rpmlint output clean
- package named according to package naming guidelines
- spec filename matches %{name}
- package meets packaging guidelines
- package licensed with open source compatible license
- license matches actual license
- spec written in American english
X spec file is not legible, see comment #1 above.
  - sed command obfuscated by using comment character (#) as delimeter
- source match upstream
a83fbf5e2e7ffd22219c513cfefe6b52  Services_Weather-1.4.0.tgz
- successfully compiles and builds on FC6 x86_64
- all build dependencies listed in BR
- no locales
- no shared libraries
- not relocatable
- owns all directories it creates
- no duplicates in %files
- file permissions set properly
- contains proper %clean section
- macro usage consistent
  - although you use %{__sed} when you dont use macros for any other system command
- contains code
- no large documentation
- files in %doc do not affect run time
- no header files or static libraries
- no pkgconfig files
- no devel subpackage required
- no .la files
- not a GUI app needing a .desktop file
- does not own files or directories owned by other packages

==== MUST ====
- Unobfuscate sed command by using @ instead of # as delimiter (# is used as a
comment character in spec files)

==== SHOULD ====
- Change %{__sed} to just "sed" to have consistent macro usage, macros for
system commands are only needed for %post/%pre,%postun/%preun sections

Comment 3 Remi Collet 2006-11-18 18:54:03 UTC
Hi, thanks for the review.

Spec URL: http://remi.collet.free.fr/rpms/extras/php-pear-Services-Weather.spec
SRPM URL:
http://remi.collet.free.fr/rpms/extras/php-pear-Services-Weather-1.4.0-2.fc7.src.rp

%changelog
* Sat Nov 18 2006 Remi Collet <Fedora> 1.4.0-2
- Unobfuscate sed command



Comment 5 Christopher Stone 2006-11-18 20:18:51 UTC
Looks good, APPROVED.

Comment 6 Remi Collet 2010-05-13 07:53:09 UTC
Package Change Request
======================
Package Name: php-pear-Services-Weather
New Branches: EL-6
Owners: remi

Comment 7 Dennis Gilmore 2010-05-13 22:39:43 UTC
CVS Done


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