Bug 476374 - Review Request: python-oasa - python library for manipulation of chemical formats
Summary: Review Request: python-oasa - python library for manipulation of chemical for...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: David Cantrell
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: BkChem
TreeView+ depends on / blocked
 
Reported: 2008-12-13 19:38 UTC by Henrique C. S. Junior
Modified: 2013-01-10 04:58 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-01-07 21:49:57 UTC
Type: ---
Embargoed:
jkeating: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Henrique C. S. Junior 2008-12-13 19:38:37 UTC
Spec URL: http://lspooky.fedorapeople.org/oasa/0.12.5/oasa.spec
SRPM URL: http://lspooky.fedorapeople.org/oasa/0.12.5/oasa-0.12.5-1.fc10.src.rpm
Description: OASA is a python library that handles chemical formats. OASA is, also, a dependency for the BKChem package (Bug 452413) that is, too, in approval process.

Comment 1 Jesse Keating 2008-12-13 21:08:01 UTC
Taking review.

Comment 2 Jesse Keating 2008-12-13 21:20:42 UTC
* Python defines correct
* Name not correct.  This is a python module, and should be named python-oasa.  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Addon_Packages_.28python_modules.29
* Licensing not correct.  Header files have 'or any later version' in them, therefore license should be GPLV2+
  oasa/elements.py - No license header
  oasa/subsearch_data.py - No license header
  oasa/tests.py - No license header
  oasa/graph/multi_thread.py - No license header
* Summary could drop the "OASA is a"
* URL is OK
* Source matches upstream
* Buildroot fine
* Requires on python is redundant, don't put that in.
* Description is OKish.
* Consistent macro usage
* Correct cleaning at %install
* Good %clean section
* defatters in %files
* %files is a bit broad.  I'd rather see something more like http://fedoraproject.org/wiki/Packaging/Python#Byte_Compiled_Files
* changelog should have a revision listed.
* rpmlint output:
  oasa.noarch: W: no-documentation
  oasa.noarch: E: description-line-too-long OASA is a free python library for   manipulating and analyzing chemical structures.
  oasa.noarch: W: incoherent-version-in-changelog 0.12.5 ['0.12.5-1.fc11', '0.12.5-1']
  oasa.src: W: mixed-use-of-spaces-and-tabs (spaces: line 12, tab: line 3)

** SUMMARY **
Fix license, report unlicensed files to upstream
Fix name
Fix summary
Fix Requires
Potentially fix %files
Fix rpmlint output

Comment 3 Henrique C. S. Junior 2008-12-13 23:08:42 UTC
Hello, Jesse, thanks for reviewing the package.

* Fix license, report unlicensed files to upstream - DONE and I already warned Beda about the files without license.
* Fix name - DONE
* Fix summary - DONE
* Fix Requires - DONE
* Potentially fix %files - I still have little experience in packaging, I hope I have done correctly this time.
* Fix rpmlint output - rpmlint is no longer showing any errors.

SPEC - http://lspooky.fedorapeople.org/oasa/0.12.5/python-oasa.spec
SRPM - http://lspooky.fedorapeople.org/oasa/0.12.5/python-oasa-0.12.5-1.fc10.src.rpm

Comment 4 Jesse Keating 2008-12-16 01:22:52 UTC
Looks good now, approving.

Comment 5 Mamoru TASAKA 2008-12-16 06:05:50 UTC
At least this won't build on dist-f11 because F-11 uses python 2.6.
I will do further check if needed.

Comment 6 Jesse Keating 2008-12-16 07:31:56 UTC
Actually I did the test build in a chroot of rawhide (f11).

Ahh, it looks like when you changed the package name, you didn't adjust the %files section since the python module name did not change:

/builddir/build/BUILDROOT/python-oasa-0.12.5-1.fc11.i386/usr/lib/python2.6/site-packages/oasa-0.12.5-py2.5.egg-info

That should be a simple fix.

Unfortunately I just noticed that Henrique is in need of a sponsor.  Henrique, have you applied for packager membership yet?

Comment 7 Henrique C. S. Junior 2008-12-16 11:23:17 UTC
No, Mamoru, I haven't applied yet. I'm doing it right now.
Coult you explain, me the problem with %files and how I fix it?

Comment 8 Mamoru TASAKA 2008-12-16 15:11:58 UTC
It seems that only fixing %files will be okay.

To Henrique:
Just replacing egg-info entry with
%{python_sitelib}/oasa-%{version}-py*.egg-info
or so will make build succeed.

Comment 10 Mamoru TASAKA 2008-12-16 16:17:18 UTC
Looks good.

Comment 11 Henrique C. S. Junior 2008-12-16 16:30:26 UTC
Thank you. 
Could you sponsor the package, Mamoru? 
I signed up in the group of packagers, but it counts as  Unapproved rather than pending. Did I something wrong or is this normal?

Comment 12 Mamoru TASAKA 2008-12-16 16:59:26 UTC
Okay, now I am sponsoring you.

Comment 13 Mamoru TASAKA 2008-12-19 16:01:14 UTC
To Henrique:

By the way, would you write CVS request? (Please follow
Join wiki)

Comment 14 Henrique C. S. Junior 2008-12-26 10:50:12 UTC
New Package CVS Request
=======================
Package Name: bkchem
Short Description: Chemicaldrawing tool
Owners: lspooky
Branches: F-9 F-10
InitialCC: jkeating mtasaka

Comment 15 Henrique C. S. Junior 2008-12-26 17:43:04 UTC
New Package CVS Request
=======================
Package Name: python-oasa
Short Description: Python library that handles chemical formats
Owners: lspooky
Branches: F-9 F-10
InitialCC: jkeating mtasaka



(In reply to comment #14)
> New Package CVS Request
> =======================
> Package Name: bkchem
> Short Description: Chemicaldrawing tool
> Owners: lspooky
> Branches: F-9 F-10
> InitialCC: jkeating mtasaka

Comment 16 Henrique C. S. Junior 2008-12-26 17:46:17 UTC
Please, forget the anterior form. I am very busy here and made a mistake in sending. The python-oasa is correct.

Comment 17 Jesse Keating 2008-12-28 00:51:05 UTC
Please don't add me to initial CC.  I'm not interested in getting bugzilla mail for this package.

Comment 18 Kevin Fenzi 2008-12-28 19:15:48 UTC
cvs done. (without jkeating in initialcc)

Comment 19 Fedora Update System 2009-01-06 10:37:33 UTC
python-oasa-0.12.5-1.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/python-oasa-0.12.5-1.fc9

Comment 20 Fedora Update System 2009-01-06 10:38:53 UTC
python-oasa-0.12.5-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/python-oasa-0.12.5-1.fc10

Comment 21 Fedora Update System 2009-01-07 09:23:04 UTC
python-oasa-0.12.5-1.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing-newkey update python-oasa'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2009-0131

Comment 22 Fedora Update System 2009-01-07 09:38:05 UTC
python-oasa-0.12.5-1.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update python-oasa'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-0239

Comment 23 Fedora Update System 2009-01-07 21:49:54 UTC
python-oasa-0.12.5-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 24 Fedora Update System 2009-01-07 21:52:39 UTC
python-oasa-0.12.5-1.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.


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