Bug 458279 - Review Request: python-arm4 - Python language bindings for Applicatioon Response Measurement
Review Request: python-arm4 - Python language bindings for Applicatioon Respo...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Till Maas
Fedora Extras Quality Assurance
:
Depends On: arm4
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-07 09:50 EDT by David Carter
Modified: 2009-04-24 19:07 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-04-24 19:07:36 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
opensource: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description David Carter 2008-08-07 09:50:16 EDT
Spec URL: http://arm4.org/Downloads/python-arm4.spec
SRPM URL: http://arm4.org/Downloads/python-arm4-1.0-1.fc9.src.rpm
Description:
Arm4 is a python module that provides for application response measurement across multiple tiers using the Open Group's Application Response Measurement (ARM) standard, Issue 4.0.

See http://www.arm4.org/ for more information.

This is the second of 3 packages I'm submitting relating to ARM, and is related to the arm4 package I submitted earlier. I'm still seeking sponsorship as I'm a new packager.
Comment 1 Till Maas 2009-04-18 11:12:28 EDT
Just two remarks I spoted:

The Source0: URL for SF.net is downloads.sourceforge... not download.sourceforge..., there is an "s" missing.

Instead of %define, %global should be used in the first line of the spec. I will try to make a full review later this day, so there is no need to update the spec yet for these two issues.
Comment 2 David Carter 2009-04-18 12:50:40 EDT
Hang on for a day. I've got an updated source to use, so I'll post an updated version later today or early tomorrow.
Comment 3 David Carter 2009-04-19 02:13:25 EDT
Updated versions:

Spec URL: http://arm4.org/Downloads/1.1-2/python-arm4.spec
SRPM URL: http://arm4.org/Downloads/1.1-2/python-arm4-1.1-2.fc9.src.rpm

These include the spec changes you suggested. Thanks for reviewing!
Comment 5 Till Maas 2009-04-21 14:27:51 EDT
[OK] rpmlint output: silent
[OK] Spec in %{name}.spec format

[OK] license allowed: EPL
[OK] license matches shortname in License: tag
[NOT OK] license in tarball and included in %doc:
COPYING should be %doc, because it specifies the license terms
a local copy of the actual license would be nice there btw.

[OK] package is code or permissive content:
{N/A} patches sent to upstream and commented

[OK] Source0 is a working URL
{OK} Sourceforge URL is Source0: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
<OK> SourceX / PatchY prefixed with %{name}
[OK] Source0 matches Upstream:
34571617ff63b28b6d28fd6d92030b04  arm4module-1.1.tar.gz

[OK] Package builds on all primary architectures:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1311916
[N/A] ExcludeArch bugs are filed and commented:
[OK] BuildRequires are complete (mock builds)
(OK) No file dependencies outside of /etc /bin /sbin /usr/bin /usr/sbin 

[N/A] %find_lang used for locales

[N/A] Every (sub)package containing libraries runs ldconfig
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig
[N/A] .h (header) files are in -devel subpackage
[N/A] .a (static libraries) are in -static subpackage
[N/A] contains .pc (pkgconfig) files and has Requires: pkgconfig
(N/A) .pc files are in -devel subpackage
[N/A] contains .so.X(.Y) files and .so is in -devel
[N/A] -devel subpackage has Requires: %{name} = %{version}-%{release}
[N/A] .la files (libtool) are not included

[N/A] Has GUI and includes %{name}.desktop
[N/A] Follows desktop entry spec
[N/A] Valid .desktop Name
[N/A] Valid .desktop GenericName
[N/A] Valid .desktop Categories
[N/A] Valid .desktop StartupNotify
[N/A] .desktop file installed with desktop-file-install in %install

[OK] Prefix: /usr not used (not relocatable)

[OK] Owns all created directories
[OK] no duplicates in %files
[OK] %defattr(-,root,root,-) is in every %files section
[OK] Does not own files or dirs from other packages
[OK] included filenames are in UTF-8

[OK] %clean is rm -rf %{buildroot} or $RPM_BUILD_ROOT 
[OK] %install starts with rm -rf %{buildroot} or $RPM_BUILD_ROOT 

[OK] Consistent macro usage

[OK] large documentation is -doc subpackage
[OK] %doc does not affect runtime

{OK} no pre-built binaries (.a, .so*, executable)
{OK} well known BuildRoot
nearly this one:
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

{OK} PreReq not used
{OK} RPM_OPT_FLAGS honoured
{OK} Useful debuginfo generated
{OK} no duplication of system libraries
{OK} no rpath
{OK} Timestamps preserved with cp and install
{N/A} Uses parallel make (%{?_smp_mflags})
{OK} Requires(pre,post) style notation not used
{OK} only writes to tmp /var/tmp $TMPDIR %{_tmppath} %{_builddir} (and %{buildroot} on %install and %clean)
{OK} no Conflicts
{OK} nothing installed in /srv
{OK} Changelog in allowed format
{OK} does not use Scriptlets
<N/A> Architecture independent packages have: BuildArch: noarch
<OK> Sane Provides: and Requires:

{OK} Follows Naming Guidelines
{NOT OK} Has BuildRequires: python
BR: is python2-devel, this should probably be python-devel

{N/A} Defines and uses %{python_sitelib}:
{OK} Defines and uses %{python_sitearch}:
%{!?python_sitearch: %global python_sitearch %(%{__python} -c "from %distutils.sysconfig import get_python_lib; print get_python_lib(1)")}

{UNCLEAR} Has BuildRequires: python-setuptools
I am not sure, whether this is required or not, but certainly it is not in the
spec.

[OK] Python eggs must be built from source. They cannot simply drop an egg from upstream into the proper directory.
[OK] Python eggs must not download any dependencies during the build process.
[OK] If egg-info files are generated by the modules build scripts they must be included in the package.
[N/A] When building a compat package, it must install using easy_install -m so it won't conflict with the main package.
[N/A] When building multiple versions (for a compat package) one of the packages must contain a default version that is usable via "import MODULE" with no prior setup.
(OK) A package which is used by another package via an egg interface should provide egg info. 

{OK} Egg install:
%install
%{__python} setup.py install --skip-build --root $RPM_BUILD_ROOT 

Other issues:
Mixed whitespace:
There are tabs uses after BR: arm4-devel and Requires: arm4, please use spaces
here or use everywhere tabs.

Please add COPYING to %doc, change python2-devel to python-devel and add BR: python-setuptools to the spec or reason, why you want to keep it the way it is and also fix the whitespace before importing this to CVS.

Because the issues found are minor, the package is APPROVED.
Comment 6 David Carter 2009-04-21 15:49:16 EDT
New Package CVS Request
=======================
Package Name: python-arm4
Short Description: Python language bindings for Application Response Measurement V4.0
Owners: grandcross
Branches: F-10 F-11
Comment 7 Kevin Fenzi 2009-04-21 16:05:43 EDT
cvs done.

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