Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: pyhunspell - Python bindings for hunspell
https://bugzilla.redhat.com/show_bug.cgi?id=514509
Summary: Review Request: pyhunspell - Python bindings for hunspell Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: opensource@till.name QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |tibbs@math.uh.edu Flag| |fedora-review?
--- Comment #1 from Jason Tibbitts tibbs@math.uh.edu 2009-08-01 17:16:16 EDT --- 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_.28p...
%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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
Björn Persson bjorn@xn--rombobjrn-67a.se changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |bjorn@xn--rombobjrn-67a.se
--- Comment #2 from Björn Persson bjorn@xn--rombobjrn-67a.se 2009-08-01 20:07:24 EDT --- 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)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
--- Comment #3 from Jason Tibbitts tibbs@math.uh.edu 2009-08-02 00:43:14 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
Till Maas opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |opensource@till.name
--- Comment #4 from Till Maas opensource@till.name 2009-08-02 04:54:41 EDT --- (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_.28p...
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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
--- Comment #5 from Jason Tibbitts tibbs@math.uh.edu 2009-08-03 20:46:31 EDT --- (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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
--- Comment #6 from Till Maas opensource@till.name 2009-08-25 16:37:47 EDT --- 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+
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #7 from Jason Tibbitts tibbs@math.uh.edu 2009-09-09 18:41:15 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
Till Maas opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #8 from Till Maas opensource@till.name 2009-09-10 08:40:42 EDT --- (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:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #9 from Jason Tibbitts tibbs@math.uh.edu 2009-09-11 16:23:52 EDT --- CVS done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
--- Comment #10 from Fedora Update System updates@fedoraproject.org 2009-09-16 19:05:49 EDT --- pyhunspell-0.1-2.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/pyhunspell-0.1-2.fc11
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
--- Comment #11 from Fedora Update System updates@fedoraproject.org 2009-09-16 19:05:59 EDT --- pyhunspell-0.1-2.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/pyhunspell-0.1-2.fc10
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |ON_QA
--- Comment #12 from Fedora Update System updates@fedoraproject.org 2009-09-18 20:05:12 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
--- Comment #13 from Fedora Update System updates@fedoraproject.org 2009-09-18 20:05:35 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
--- Comment #14 from Fedora Update System updates@fedoraproject.org 2009-10-06 06:01:57 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |0.1-2.fc11 Resolution| |ERRATA
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
--- Comment #15 from Fedora Update System updates@fedoraproject.org 2009-10-06 06:03:04 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=514509
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|0.1-2.fc11 |0.1-2.fc10
package-review@lists.fedoraproject.org