[Bug 511276] Review Request: comoonics-base-py - Comoonics minimum baselibraries written in Python

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=511276





--- Comment #6 from Nils Philippsen <nphilipp at redhat.com>  2009-07-20 05:53:50 EDT ---
First round :-). I'd appreciate if you would check the issues listed below in
your other packages pending review, this would make their reviews so much
simpler. Thanks!

Items marked "GOOD" or "PASS" fulfil the guidelines or they don't apply to this
package.
Items marked "CHECK" aren't covered by the guidelines but you should check and
fix them anyway in my opinion.
Items marked "BAD" violate the guidelines in some point and need to be fixed.

- BAD: rpmlint run on comoonics-base-py-0-1-1.src.rpm flags errors/warnings:

comoonics-base-py.src: E: description-line-too-long comoonics.ComDataObject:
abstract basic DOM-Based class that is base for any other DOM-Based class
comoonics-base-py.src: E: description-line-too-long comooncis.ComExceptions:
the library provides a base class for all comoonics exceptions.
comoonics-base-py.src: E: description-line-too-long comoonics.ComSystem:    
library for some commonly used functions to execute commands.

--> description lines must be 79 characters or shorter, please shorten (and fix
the typo "comooncis.ComExceptions:..." while you're at it)

comoonics-base-py.src: E: no-changelogname-tag

--> start and maintain a package changelog, see
https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs

comoonics-base-py.src: W: invalid-license GPL

--> I've looked at a few source files which state that they are licensed under
GPL version 3 or later, this would make the license "GPLv3+". It would be nice
if the package contained a README making this explicit for the whole package as
well as a copy of the license (since you're upstream, this should be no
problem). Be sure to bump the upstream/tarball version when you do this though.

comoonics-base-py.src: W: non-coherent-filename comoonics-base-py-0-1-1.src.rpm
comoonics-base-py-0.1-1.src.rpm

--> I think this is only some typo from copying the file over to your
webserver, correct?

comoonics-base-py.src:17: W: hardcoded-packager-tag Marc

--> please get rid of the Packager:/Vendor: lines, see
https://fedoraproject.org/wiki/Packaging/Guidelines#Tags

comoonics-base-py.src:38: W: setup-not-quiet

--> use "%setup -q" -- by the way, what's the business with
%version/%unmangled_version? As it is, they're the same and packages should
have the same version number as their upstream as well. If you're thinking
about alpha/beta versions, please see
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Version for
more info -- you (as upstream) will make your life (as packager) much easier
then. Also, using "%define name foo", then "Name: %name" and the like is
unnecessary, if you just define name, versino, release the normal way, the
corresponding macros will be set as well.

comoonics-base-py.src: E: no-cleaning-of-buildroot %install
1 packages and 0 specfiles checked; 5 errors, 4 warnings.

--> clean the build root as described in
https://fedoraproject.org/wiki/Packaging/Guidelines#Tags (just like in %clean)
before installing

- GOOD: package name according to guidelines:
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Addon_Packages_.28python_modules.29
(only fix the source rpm file name please)
- GOOD: spec file named properly
- GOOD/CHECK: licensing mostly clear (see above, an added README would be good)
and according to licensing guidelines
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines
- CHECK: license files are not shipped as documentation, but they aren't
shipped in the upstream tarball, see
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text --
since you're upstream you really should ship it though ;-)
- GOOD: the spec file is written in American English
- GOOD: the spec file is legible
- CHECK: no source tarball URL, it would be good if the tarball used were
directly available -- see https://fedoraproject.org/wiki/Packaging/SourceURL
- BAD: doesn't build in mock for x86_64/Rawhide
- BAD: no build dependencies listed

--> at least python-devel is missing as a build dependency (and a dependency of
the generated installable package), see
http://fedoraproject.org/wiki/Packaging:Python

- PASS: doesn't ship locale files
- PASS: no libraries shipped
- BAD/CHECK: package is made relocatable (Prefix: ...), please remove or
justify its use,
https://fedoraproject.org/wiki/Packaging/Guidelines#RelocatablePackages

- BAD: not all shipped directories owned by package, direct dependency or
filesystem

--> instead of using the file list generated by your setup.py, define
%python_sitelib at the top of your spec file and simply list the directory
containing your module, see http://fedoraproject.org/wiki/Packaging:Python :

%{!?python_sitelib: %global python_sitelib %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib()")}
[...]
%files
%defattr (-, root, root, -)
%{python_sitelib}/comoonics

- GOOD: no duplicates in %files
- CHECK: permissions on files

--> There are three files in the comoonics module which have mode 0644, but a
"#!/usr/bin/python" line at the top of the file:

    comoonics-base-py.noarch: E: non-executable-script
/usr/lib/python2.6/site-packages/comoonics/XmlTools.py 0644 /usr/bin/python
    comoonics-base-py.noarch: E: non-executable-script
/usr/lib/python2.6/site-packages/comoonics/DictTools.py 0644 /usr/bin/python
    comoonics-base-py.noarch: E: non-executable-script
/usr/lib/python2.6/site-packages/comoonics/ComProperties.py 0644
/usr/bin/python

    Are they supposed to be executed directly (-> fix mode) or not (remove the
line)?

- GOOD: package has a %clean section
- GOOD: package uses macros consistently
- GOOD: the package contains code, not content
- PASS: no large documentation files
- GOOD: %doc doesn't affect runtime
- PASS: no header files
- PASS: no static libraries
- PASS: no pkgconfig files
- PASS: no libraries included
- PASS: no devel package
- GOOD: no *.la libtool archives
- PASS: no desktop file
- GOOD: doesn't own files or directories owned by other packages
- BAD: build root isn't cleaned at the beginning of %install (see rpmlint
comment above)
- GOOD: all file names are valid UTF-8

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.