Bug 484808 - Review Request: python-linux-procfs - parser classes for information found in /proc
Summary: Review Request: python-linux-procfs - parser classes for information found in...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: python-linux-procfs
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Arnaldo Carvalho de Melo
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-02-10 00:24 UTC by Arnaldo Carvalho de Melo
Modified: 2009-02-19 01:51 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-02-19 01:51:47 UTC
Type: ---
Embargoed:
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Arnaldo Carvalho de Melo 2009-02-10 00:24:03 UTC
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-10 04:38:22 UTC
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 13:03:16 UTC
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 14:13:56 UTC
(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 15:28:16 UTC
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 15:53:55 UTC
(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 20:32:04 UTC
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 21:20:47 UTC
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 07:14:07 UTC
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 16:24:04 UTC
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 19:57:35 UTC
cvs done.


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