Bug 557948
Summary: | Review Request: PyAIML - A Python AIML Interpreter | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Sebastian Dziallas <sebastian> |
Component: | Package Review | Assignee: | Susi Lehtola <susi.lehtola> |
Status: | CLOSED WONTFIX | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, notting, pbrobinson, spacewar, susi.lehtola |
Target Milestone: | --- | Flags: | susi.lehtola:
fedora-review?
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2010-02-10 22:43:19 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 182235, 462625, 558617 |
Description
Sebastian Dziallas
2010-01-22 21:15:12 UTC
I'm not yet sponsored as a packager, but here's an unofficial review: rpmlint output: 1 packages and 1 specfiles checked; 0 errors, 0 warnings. source tarball verified against upstream compiles and builds on F12 x86_64 and i386 using mock verified that upstream is public domain, so no license text file needed When I try to verify the functionality of the package using the example in the README.txt, and get to the point of doing 'k.respond("load aiml b")', I get 'WARNING: No match found for input: load aiml b'. I don't really know anything about AIML so I'm not certain what is going wrong, but it doesn't match the expected behaviour. I'm not sure about this one, but none of the examples in the Packaging:Python guidelines include the whole sitelib directory, so rather than having the files section contain %{python_sitelib}/*, perhaps it might be better to have: %{python_sitelib}/%{name}-%{version}-*.egg-info %{python_sitelib}/aiml/ Assigning. I'm going to sponsor Eric, here's a verbose review: rpmlint output is clean. MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK - Be sure to check for older reviews on the review queue, first (or that if the package already is in Fedora). MUST: The spec file for the package is legible and macros are used consistently. OK - Spec files are legible, no mixing of styles ($RPM_BUILD_ROOT vs %{buildroot}), macro usage is sane. MUST: The package must be named according to the Package Naming Guidelines. OK - Naming guidelines state that Python libraries must be named python-foo, unless the name is pyfoo or Pyfoo. MUST: The spec file name must match the base package %{name}. OK MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. OK MUST: The License field in the package spec file must match the actual license. NEEDSWORK - Nothing in the tarball actually specifies a license or defines the code to be in the public domain. - All I can find is License: UNKNOWN in PKG-INFO. - Upstream needs to add a license statement in the tarball before this package can be approved. MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. NEEDSWORK - Source URL is malformed: $ spectool -g PyAIML.spec (clip) 2010-01-24 12:08:58 (117 KB/s) - “./index.html” saved [65180/65180] - Source URL does not adhere to source URL guidelines http://fedoraproject.org/wiki/Packaging/SourceURL - Correct Source URL is http://downloads.sourceforge.net/pyaiml/PyAIML%20%28unstable%29/PyAIML-%{version}.tar.bz2 MUST: The package MUST successfully compile and build into binary rpms. OK MUST: The spec file MUST handle locales properly. N/A MUST: Optflags are used and time stamps preserved. OK MUST: Packages containing shared library files must call ldconfig. N/A MUST: A package must own all directories that it creates or require the package that owns the directory. OK - As Eric stated in comment #1, be more verbose in %files since you might miss if the egg is not built for some reason or another. MUST: Files only listed once in %files listings. OK MUST: Debuginfo package is complete. OK MUST: Permissions on files must be set properly. OK MUST: Clean section exists. OK MUST: Large documentation files must go in a -doc subpackage. N/A MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. ~OK - You could add TODO.txt to %doc. MUST: Header files must be in a -devel package. N/A MUST: Static libraries must be in a -static package. N/A MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A MUST: Packages does not contain any .la libtool archives. N/A MUST: Desktop files are installed properly. N/A MUST: No file conflicts with other packages and no general names. OK MUST: Buildroot cleaned before install. OK SHOULD: %{?dist} tag is used in release. OK SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. NEEDSWORK - No license file is included, please ask upstream to include one. SHOULD: The package builds in mock. OK **** The source URL and egg stuff are minor issues. The license issue is bigger: there is no license specified, so we can't include this in Fedora - blocking FE-LEGAL. If upstream e.g. adds in README.TXT (or in the comment of the source code files) a statement "This code is in the public domain." everything would be OK. However, the latest (unstable!) release has been in 2004, so I don't have high hopes of this happening... D'oh! You're right, indeed. I've pinged the upstream author to see if we've any chances there. Thanks for the review already. Argh! Upstream is not responsive, so I guess I'll have to close this for now. If he replies, I'll reopen here. |