Bug 442377 - Review Request: python-pysctp - Python binding for the SCTP network protocol
Review Request: python-pysctp - Python binding for the SCTP network protocol
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rakesh Pandit
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2008-04-14 12:12 EDT by Neil Horman
Modified: 2008-09-11 13:21 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-09-11 13:21:25 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rpandit: fedora‑review+
tibbs: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Neil Horman 2008-04-14 12:12:42 EDT
Spec URL: http://nhorman.fedorapeople.org/python-pysctp-0.3.1-1.fc8.src.rpm
SRPM URL: http://nhorman.fedorapeople.org/python-pysctp.spec
Description: python-pysctp is the python binding code for the SCTP network protocol.   SCTP uses a set of extensions to the standard BSD socket api, and this pythhon module exposes those extensions to the python programming language, making the protocol useable
Comment 1 Neil Horman 2008-08-05 12:17:46 EDT
ping, can someoneae please review this?  Thanks!
Comment 2 Rakesh Pandit 2008-09-09 14:33:21 EDT
Build successfully

[rpmbuild@rocky SRPMS]$ rpmlint python-pysctp-0.3.1-1.fc9.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[rpmbuild@rocky i386]$ rpmlint python-pysctp-debuginfo-0.3.1-1.fc9.i386.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[rpmbuild@rocky i386]$ rpmlint -i python-pysctp-0.3.1-1.fc9.i386.rpm 
python-pysctp.i386: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

python-pysctp.i386: E: devel-dependency lksctp-tools-devel
Your package has a dependency on a devel package but it's not a devel package

I guess you can ignore this one.

1 packages and 0 specfiles checked; 1 errors, 1 warnings.

Requires and BuildRequires have lot of repeated dependencies

python-setuptools-devel will provide you python & python-setuptools so you can remove both.

Once you update I will do a detailed review.
Comment 3 Rakesh Pandit 2008-09-09 14:34:26 EDT
You should include doc files like draft in doc folder, README (it has some essential info) ChangeLog
Comment 4 Neil Horman 2008-09-09 16:30:52 EDT
New files, with issues above addressed:
Spec URL: http://nhorman.fedorapeople.org/python-pysctp-0.3.1-2.fc8.src.rpm
SRPM URL: http://nhorman.fedorapeople.org/python-pysctp.spec
Comment 5 Rakesh Pandit 2008-09-10 01:18:57 EDT
Your link to spec has old spec file. I am using srpm which has latest spec though :)
Comment 6 Rakesh Pandit 2008-09-10 01:26:49 EDT
python-pysctp.i386: W: incoherent-version-in-changelog 0.3.1-1 0.3.1-2.fc9
The last entry in %changelog contains a version identifier that is not
coherent with the epoch:version-release tuple of the package.

1 packages and 0 specfiles checked; 1 errors, 1 warnings.

You have forgotten to update the changeLog. Please do it.

python-pysctp.i386: E: devel-dependency lksctp-tools-devel
Comment 7 Rakesh Pandit 2008-09-10 02:37:22 EDT

1. Fix changelog entry before importing
2. Please look at some details mentioned below before requesting for CVS and importing


[!] rpmlint -- pasted above. Correctly fill the changelog entry
[x] Naming -- Okay, we already have package names like python-pydns python-pyspf python-pycurl

    You can however keep pysctp also because it is an upstream name and is treated as an exception in
python naming convention

It is your choice. But make this choice before requesting for CVS :)

[x] md5sum

Source downloaded from project hosting
[rpmbuild@rocky python]$ md5sum pysctp-0.3.1.tar.gz 
7fd43fa7a7f31cff557d0a69fa751e5f  pysctp-0.3.1.tar.gz

Source from srpm
[rpmbuild@rocky python-pysctp-0.3.1-2.fc8]$ md5sum pysctp-0.3.1.tar.gz 
7fd43fa7a7f31cff557d0a69fa751e5f  pysctp-0.3.1.tar.gz


[x] %files section - includes all files

In spite of using {python_sitelib}/* you may consider using {python_sitelib}/<modulename>/<regex> which is better option. This is optional though, but what you are doing is bad practice. 

[x] license - fedora approved
[x] license field - matches actual license
[x] source does not have license file - may be requested upstream
[?] eggs -

Check whether eggs are handled correctly in F-8, you may like to have a look at 

My mock seems not working right now so could not check for f-8 specifically. But it seems to me okay - looking at spec

[x] spec file legible and written in American English
[x] source URL correct
[x] Builds successfully on koji
[x] owns all directories created
[x] %makeinstall %clean rm -rf $RPM_BUILD_ROOT
[NA] dev, header file, static libs 

Optional suggestions:
[x] may be you could request upstream about including license file
[!] Description could be bit  better in my view.  

*You may like to ignore them*

Key NA = N/A, x = Check, ! = Problem, ? = Not evaluated
Comment 8 Neil Horman 2008-09-10 08:42:40 EDT
Thank you!

I've got a fixed up version of the package here, I'll be keeping the python-pysctp naming conventions, and I've verified that it builds on all arches.  I'll inquire upstream about the need for an explicit copy of the LGPLv2 license file shortly.  Thanks!
Comment 9 Neil Horman 2008-09-10 08:47:24 EDT
New Package CVS Request
Package Name: python-pysctp
Short Description: python bindings for the SCTP sockets extension api
Owners: nhorman
Branches: devel, F-9, EL-4, EL-4
InitialCC: nhorman@redhat.com
Comment 10 Jason Tibbitts 2008-09-10 20:05:19 EDT
You listed EL-4 twice.  I assumed you meant EL-5 for one of them.

InitialCC needs to be a Fedora account, but it is pointless to CC yourself on a package you own.  I've just skipped that line.

CVS done.

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