Bug 215200 (pear-Svcs-Weather)
| Summary: | Review Request: php-pear-Services-Weather - This class acts as an interface to various online weather-services | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Remi Collet <fedora> |
| Component: | Package Review | Assignee: | Christopher Stone <chris.stone> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | Flags: | dennis:
fedora-cvs+
|
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2006-11-22 06:19:14 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | 214236 | ||
| Bug Blocks: | 163779 | ||
|
Description
Remi Collet
2006-11-12 10:31:34 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
==== 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
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 Looks good, APPROVED. Package Change Request ====================== Package Name: php-pear-Services-Weather New Branches: EL-6 Owners: remi CVS Done |