This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 215200 - (pear-Svcs-Weather) Review Request: php-pear-Services-Weather - This class acts as an interface to various online weather-services
Review Request: php-pear-Services-Weather - This class acts as an interface t...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christopher Stone
Fedora Package Reviews List
:
Depends On: php-pear-SOAP
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-11-12 05:31 EST by Remi Collet
Modified: 2010-05-13 18:39 EDT (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-11-22 01:19:14 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
dennis: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Remi Collet 2006-11-12 05:31:34 EST
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 12:15:35 EST
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 12:39:51 EST
==== 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 13:54:03 EST
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@famillecollet.com> 1.4.0-2
- Unobfuscate sed command

Comment 5 Christopher Stone 2006-11-18 15:18:51 EST
Looks good, APPROVED.
Comment 6 Remi Collet 2010-05-13 03:53:09 EDT
Package Change Request
======================
Package Name: php-pear-Services-Weather
New Branches: EL-6
Owners: remi
Comment 7 Dennis Gilmore 2010-05-13 18:39:43 EDT
CVS Done

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