| Summary: | Review Request: ast - A Library for Handling World Coordinate Systems in Astronomy | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Orion Poplawski <orion> |
| Component: | Package Review | Assignee: | Sergio Pascual <sergio.pasra> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fedora-package-review, notting, sergio.pasra |
| Target Milestone: | --- | Flags: | sergio.pasra:
fedora-review+
gwync: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2011-12-05 21:47:31 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
|
Description
Orion Poplawski
2011-05-03 22:00:44 UTC
Hi, some comments: * The description is too long. I haven't found a guideline about this but IMHO with less than 10 lines is enough. Consider that $ rpm -qi ast outputs almost two screens full of text, hiding the rpm information. * The upstream version of the package is 5.6-0. What do you thonk of translating this to 5.6.0-1 instead of 5.6-1? If upstream releases 5.6-3 you will have to edit the Source macro to get the correct source. (Weird versioning, by the way) * Source should contain a full URL * Everything inside /usr/share/ast is documentation and is not needed to work with the libraries. As such, I think the contents should go to %docs * Furthermore, the docs are about 40 M in size. Removing the .tex files reduces the size to around 25 M. These files are good candidates to go to a ast-doc package. If you don't want to make a separate doc package, the library documentation should be in -devel subpackage. When I have more time, I will see if the Makefile can be patched to "remove the unresolved symbol" warnings. * Fri Oct 14 2011 Orion Poplawski <orion.com> 5.7.2-1 - Update to 5.7-2 - Truncate description - Move documentation to subpackage I'm not sure how to get a usable URL. I put it in as a comment in the spec. http://www.cora.nwra.com/~orion/fedora/ast.spec http://www.cora.nwra.com/~orion/fedora/ast-5.7.2-1.fc15.src.rpm Sergio - Are you still up for doing this review, or should I put it back in the queue? I would like to do the review this weekend... I've been very busy these months These are blockers, after fixing them the package should be ok. License ------- * There are some files proj.c proj.h wcsmath.h wcstrig.c wcstrig.h (from an old version of wcslib) under LGPLv2+. So the license tag should be: GPLv2+ and LGPLv2+ * The FSF address is wrong. You should report it upstream Libraries --------- * In the installed package there are some pgplot related libraries. As pgplot is not free and can't be distributed by Fedora, I suggest to remove the libraries /usr/lib64/libast_pgplot3d.so /usr/lib64/libast_pgplot3d.so.0 /usr/lib64/libast_pgplot3d.so.0.0.0 /usr/lib64/libast_pgplot.so /usr/lib64/libast_pgplot.so.0 /usr/lib64/libast_pgplot.so.0.0.0 Documentation ------------- * rpmlint complains about hidden directories and empty files. They should be removed. ast-doc.x86_64: W: hidden-file-or-dir /usr/share/doc/ast/sun210.htx/.star2html-init ast-doc.x86_64: E: zero-length /usr/share/doc/ast/sun210.htx/star2html.sty ast-doc.x86_64: E: zero-length /usr/share/doc/ast/sun211.htx/star2html.sty ast-doc.x86_64: W: hidden-file-or-dir /usr/share/doc/ast/sun211.htx/.star2html-init * Tex files are included. They should be removed unless they are needed for something. * In the spec, you create a /usr/share/doc/ast directory and move there the documentation installed. But the name of the directory is incorrect. Everything in docdir is named "package-version", in this case ast-doc-5.7.2 I would let rpm do its job and grab the docs from the source dir %files doc %doc sun210.htx sun210.ps sun211.htx sun211.ps Other recommendations, not blockers * The following macros are not needed anymore, so you can remove them safely from the specfile: BuildRoot, %clean and %defattr * If you are willing and have time, you can try to educate upstream about: - its weird versioning scheme - linking to wcstools instead of pasting wcstools files in the code - using pkgconfig instead of home-made solutions - creating pgplot libraries only if pgplot is present http://www.cora.nwra.com/~orion/fedora/ast.spec http://www.cora.nwra.com/~orion/fedora/ast-6.0.1-1.fc16.src.rpm * Mon Nov 28 2011 Orion Poplawski <orion.com> 6.0.1-1 - Update to 6.0-1 - Fixup some lib linkages - Fix license tag - Fix FSF license - Fixup doc install - Drop BuildRoot, clean, defattr I've sent an email upstream about the various issues. Rpmlint output:
ast.src:12: W: macro-in-comment %{srcver}
ast.src: W: invalid-url Source0: ast-6.0-1.tar.gz
ast-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/ast-6.0-1/loader.h
ast-devel.x86_64: W: spelling-error %description -l en_US prog -> prig, prof, pro
ast-devel.x86_64: W: no-documentation
ast-devel.x86_64: W: no-manual-page-for-binary ast_link
Package and spec are named according the guidelines
License is GPLv2+ and LGPLv2+
Specfile legible
Source matches upstream, source follows the convention for Troublesome URLs (see http://fedoraproject.org/wiki/Packaging/SourceURL)
Package builds, No ExcludeArch needed
BuildRequires listed
ldconfig called properly
No bundled libraries (the files from wcslib have been modified by ast developers, so I don't consider it to be the same library.)
Owns directories it creates
Macros are consistent
Large docs are in -doc subpackage
Headers and .so are in -devel subpackage
-devel requires base package
No .la files
BuildRoot is not needed
%clean is not needed
Package is APPROVED
By the way, you are not required to fix the incorrect FSF address. But if you do, please fix also loader.h before uploading
Thanks for the review. New Package SCM Request ======================= Package Name: ast Short Description: A Library for Handling World Coordinate Systems in Astronomy Owners: orion Branches: f15 f16 el6 InitialCC: Git done (by process-git-requests). Checked in and built. Thanks Jon. |