Bug 514509 - Review Request: pyhunspell - Python bindings for hunspell
Summary: Review Request: pyhunspell - Python bindings for hunspell
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-07-29 13:35 UTC by Till Maas
Modified: 2009-10-06 10:03 UTC (History)
4 users (show)

Fixed In Version: 0.1-2.fc10
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-10-06 10:02:03 UTC
tibbs: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Till Maas 2009-07-29 13:35:58 UTC
Spec URL: http://till.fedorapeople.org/review/pyhunspell.spec
SRPM URL: http://till.fedorapeople.org/review/pyhunspell-0.1-1.fc10.src.rpm
Description: Python bindings for hunspell

Comment 1 Jason Tibbitts 2009-08-01 21:16:16 UTC
Builds fine;  rpmlint says:
  pyhunspell.x86_64: W: no-documentation
which is fine, since there isn't any.

I'm unsure of the name.  The upstream site calls itself "pyhunspell" but the tarball and the module are called hunspell.  The guidelines only say "when in doubt, use the name of the module that you type to import it in a script", which would be "hunspell" (and to prepend "python-" if "py" isn't in the name).  Not really sure what's correct here.
http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28python_modules.29

%description could use a period.

I note that the compiler flags all appear twice; I think setup.py build gets them right without having them passed, but I'm not certain of it.

* source files match upstream.  sha256sum:             
   ec1bfa633f937b67f6b2a7134ee2600aecf704a62042e2dc9f0eb4a2ec18c67d
   hunspell-0.1.tar.gz
? unsure of the package name.
* specfile is cleanly written and uses macros consistently.
* summary is OK.                                                              
* description is OK (could use a period).
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
   hunspell.so()(64bit)
   pyhunspell = 0.1-1.fc12
   pyhunspell(x86-64) = 0.1-1.fc12
  =
   libhunspell-1.2.so.0()(64bit)
   libpython2.6.so.1.0()(64bit)
   python(abi) = 2.6

* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

Comment 2 Björn Persson 2009-08-02 00:07:24 UTC
1: The license tag is wrong. It says "GPLv3+", but in hunspell.c it
says "the GNU Library General Public License [...] either version 3 of the
License, or (at your option) any later version". Although it's officially
"Lesser", not "Library", it's still clearly LGPLv3+. It also says "GNU Lesser
General Public License" on the website.

2: There is no license file. Have you asked upstream to include a copy of the LGPL version 3 in the tarball?
(https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text)

Comment 3 Jason Tibbitts 2009-08-02 04:43:14 UTC
Bjorn's right about the licence; I grepped for lesser as usual and of course didn't find it.  You can bug upstream for a copy of the license text if you like; that's your business.  I don't find it productive to say that for well over half of the packages I see which don't bother to include license text.

Comment 4 Till Maas 2009-08-02 08:54:41 UTC
(In reply to comment #1)

> I'm unsure of the name.  The upstream site calls itself "pyhunspell" but the
> tarball and the module are called hunspell.  The guidelines only say "when in
> doubt, use the name of the module that you type to import it in a script",
> which would be "hunspell" (and to prepend "python-" if "py" isn't in the name).
>  Not really sure what's correct here.
> http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28python_modules.29

Imho pyhunspell is ok here, because the project calls itself pyhunspell. But I asked the packaging list to be sure:
https://www.redhat.com/archives/fedora-packaging/2009-August/msg00002.html


> %description could use a period.
But it is not a complete sentence.

 
> I note that the compiler flags all appear twice; I think setup.py build gets
> them right without having them passed, but I'm not certain of it.

They come from rpmdev-newspec -t python, maybe they are needed for EPEL.


(In reply to comment #3)
> Bjorn's right about the licence; I grepped for lesser as usual and of course
> didn't find it.  You can bug upstream for a copy of the license text if you
> like; that's your business.  I don't find it productive to say that for well
> over half of the packages I see which don't bother to include license text.  

I will the change the license tag in the spec before importing it. Here is a ticket to include the license text in the tarball and handle the other issues:
http://code.google.com/p/pyhunspell/issues/detail?id=1

Comment 5 Jason Tibbitts 2009-08-04 00:46:31 UTC
(In reply to comment #4)

> Imho pyhunspell is ok here, because the project calls itself pyhunspell. But I
> asked the packaging list to be sure:
> https://www.redhat.com/archives/fedora-packaging/2009-August/msg00002.html

I happen to disagree, but it isn't a big deal.  Actually I disagree with the whole "py" exception, but I guess it's too late to change that now.  FPC can talk about it on Wednesday.

> But it is not a complete sentence.

That suggests a fix.  %description should be a paragraph describing the package, or a sentence at minimum.

> They come from rpmdev-newspec -t python, maybe they are needed for EPEL.

They aren't.  That's just a template; you should change it to suit the package.

Comment 6 Till Maas 2009-08-25 20:37:47 UTC
I did not hear from upstream, so here is an updated spec and srpm:

Spec URL: http://till.fedorapeople.org/review/pyhunspell.spec
SRPM URL: http://till.fedorapeople.org/review/pyhunspell-0.1-2.fc10.src.rpm

- Remove CFLAGS, which are used automagically
- Change the %%description to a full sentence
- Adjust the license tag, it's actually LGPLv3+

Comment 7 Jason Tibbitts 2009-09-09 22:41:15 UTC
Sorry for my late reply; the semester has started and now that the students are back I have much less free time.

Everything looks good to me.  The license tag is now correct, although I still can't fathom why someone would go to the trouble of using the proper license block in the code except for changing "Lesser" to "Library".  %description looks fine and everything still builds OK.

APPROVED

Comment 8 Till Maas 2009-09-10 12:40:42 UTC
(In reply to comment #7)
> Sorry for my late reply; the semester has started and now that the students are
> back I have much less free time.

No worries, this fits perfectly into my schedule.
 
> Everything looks good to me.  The license tag is now correct, although I still
> can't fathom why someone would go to the trouble of using the proper license
> block in the code except for changing "Lesser" to "Library".  %description
> looks fine and everything still builds OK.

I guess the author just copied and pasted it from some older code when the license was still called "Lesser". Thank you and Björn for the review.


New Package CVS Request
=======================
Package Name: pyhunspell
Short Description: Python bindings for hunspell
Owners: till
Branches: F-10 F-11 EL-5
InitialCC:

Comment 9 Jason Tibbitts 2009-09-11 20:23:52 UTC
CVS done.

Comment 10 Fedora Update System 2009-09-16 23:05:49 UTC
pyhunspell-0.1-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/pyhunspell-0.1-2.fc11

Comment 11 Fedora Update System 2009-09-16 23:05:59 UTC
pyhunspell-0.1-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/pyhunspell-0.1-2.fc10

Comment 12 Fedora Update System 2009-09-19 00:05:12 UTC
pyhunspell-0.1-2.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 pyhunspell'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-9701

Comment 13 Fedora Update System 2009-09-19 00:05:35 UTC
pyhunspell-0.1-2.fc11 has been pushed to the Fedora 11 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 pyhunspell'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-9706

Comment 14 Fedora Update System 2009-10-06 10:01:57 UTC
pyhunspell-0.1-2.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 15 Fedora Update System 2009-10-06 10:03:04 UTC
pyhunspell-0.1-2.fc10 has been pushed to the Fedora 10 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.