Bug 484808 - Review Request: python-linux-procfs - parser classes for information found in /proc
Review Request: python-linux-procfs - parser classes for information found in...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: python-linux-procfs (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Arnaldo Carvalho de Melo
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-09 19:24 EST by Arnaldo Carvalho de Melo
Modified: 2009-02-18 20:51 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-02-18 20:51:47 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Arnaldo Carvalho de Melo 2009-02-09 19:24:03 EST
Spec URL: http://fedorapeople.org/~acme/fedora/python-linux-procfs/python-linux-procfs.spec
SRPM URL: http://fedorapeople.org/~acme/fedora/python-linux-procfs/python-linux-procfs-0.4.3-1.fc10.src.rpm
Description: Abstractions to extract information from the Linux kernel /proc files.
Comment 1 Parag AN(पराग) 2009-02-09 23:38:22 EST
Before starting review
1)just wonder why linux word used in project name? Can you drop that name and make it upstream also like python-procfs?
2) I can't see any license text in .py files and any separate license text file.
Comment 2 Arnaldo Carvalho de Melo 2009-02-10 08:03:16 EST
1) Because procfs also exists in other OSes, such as Solaris, where this idea, AFAIK, came from. And the files parsed have very much Linux specific keys and formats, so having "linux" in the name is needed.

2) All the .py files state that the files are under GPLv2, as does the specfile License tag. Is it really a requirement that even with this clearly stated we need a copy of the license in a LICENSE named file?

If that is the case, sure, I can do it, and will as well as to submit a patch for rpmlint to warn about that :-)
Comment 3 Parag AN(पराग) 2009-02-10 09:13:56 EST
(In reply to comment #2)
> 1) Because procfs also exists in other OSes, such as Solaris, where this idea,
> AFAIK, came from. And the files parsed have very much Linux specific keys and
> formats, so having "linux" in the name is needed.
> 
That is ok then.

> 2) All the .py files state that the files are under GPLv2, as does the specfile
> License tag. Is it really a requirement that even with this clearly stated we
> need a copy of the license in a LICENSE named file?
> 
> If that is the case, sure, I can do it, and will as well as to submit a patch
> for rpmlint to warn about that :-)

SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. 

I will say that either you can add license text as header in all .py files or include separate license text file.
see http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text

you can then include license file as %doc


3) F6 is already EOL and since F7 we have python >= 2.5 so you can change 

%if "%{python_ver}" >= "2.5"
%{python_sitelib}/*.egg-info
%endif
to
%{python_sitelib}/*.egg-info
only
Comment 4 Arnaldo Carvalho de Melo 2009-02-10 10:28:16 EST
Ok, I'll add the LICENSE text and will add is as %doc.

3) That is myself trying to share the same specfile with other distros, from Red Hat, that still don't have eggs, like RHEL5 and 4 :-) If there is no problem in leaving this, I'd prefer to have just one spec file.

Do you have any other suggestions before I tag 0.4.4 and push a new tarball out?
Comment 5 Parag AN(पराग) 2009-02-10 10:53:55 EST
(In reply to comment #4)
> Ok, I'll add the LICENSE text and will add is as %doc.
> 

==> thanks.

> 3) That is myself trying to share the same specfile with other distros, from
> Red Hat, that still don't have eggs, like RHEL5 and 4 :-) If there is no
> problem in leaving this, I'd prefer to have just one spec file.
==>That is valid reason then. you can keep if..endif

> 
> Do you have any other suggestions before I tag 0.4.4 and push a new tarball
> out?

===> yes. check rpmlint on binary rpm

python-linux-procfs.noarch: E: script-without-shebang /usr/lib/python2.5/site-packages/python_linux_procfs-0.1-py2.5.egg-info

you need to chmod 644 /usr/lib/python2.5/site-packages/python_linux_procfs-0.1-py2.5.egg-info
Comment 6 Arnaldo Carvalho de Melo 2009-02-10 15:32:04 EST
I did that and fixed it for the ones that comes from the tarball, but didn't knew how to silence the one about the eggs, will do, tag 0.4.4.
Comment 7 Arnaldo Carvalho de Melo 2009-02-10 16:20:47 EST
Here it goes, hopefully doing everything we agreed upon here:

Spec URL:
http://fedorapeople.org/~acme/fedora/python-linux-procfs/python-linux-procfs.spec
SRPM URL:
http://fedorapeople.org/~acme/fedora/python-linux-procfs/python-linux-procfs-0.4.4-1.fc10.src.rpm

Thanks a lot!
Comment 8 Parag AN(पराग) 2009-02-11 02:14:07 EST
Review:
+ package builds in mock (rawhide i386).
koji build => http://koji.fedoraproject.org/koji/taskinfo?taskID=1118831
+ rpmlint is silent for SRPM and for RPM.
+ source files match upstream.
7209283a013e3358bad7dc4c3b19c109  python-linux-procfs-0.4.4.tar.bz2
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc files present.
+ BuildRequires are proper.
+ defattr usage is correct.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code.
+ no static libraries.
+ no .pc file present.
+ no -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no scriptlets are used.
+ Not a GUI app.

Suggestions:
1) According to COPYING file license tag should be GPLv2+
from http://fedoraproject.org/wiki/Licensing#Good_Licenses, you have included license text that matches GPLv2+

APPROVED.
Comment 9 Arnaldo Carvalho de Melo 2009-02-17 11:24:04 EST
New Package CVS Request
=======================
Package Name: python-linux-procfs
Short Description: Abstractions to extract information from the Linux kernel /proc
files.
Owners: acme
Branches: F11
InitialCC:
Comment 10 Kevin Fenzi 2009-02-18 14:57:35 EST
cvs done.

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