Bug 476357
| Summary: | Review Request: libicns - Library for manipulating Macintosh icns files | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Andrea Musuruane <musuruan> | ||||
| Component: | Package Review | Assignee: | Conrad Meyer <cse.cem+redhatbugz> | ||||
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
| Severity: | medium | Docs Contact: | |||||
| Priority: | medium | ||||||
| Version: | rawhide | CC: | cse.cem+redhatbugz, fedora-package-review, notting | ||||
| Target Milestone: | --- | Flags: | cse.cem+redhatbugz:
fedora-review+
kevin: 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: | 2008-12-21 14:57:29 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: | |||||||
| Attachments: |
|
||||||
|
Description
Andrea Musuruane
2008-12-13 14:07:10 UTC
Created attachment 327293 [details]
[Review]
Attached is my initial review.
General thoughts:
- Why have a separate -utils package? Generally we put libs and binaries in the same package. (I could see the main package Provides:-ing a -utils subpackage.)
> - [ ??? ] MUST: The package must meet the Packaging Guidelines. > Why -utils subpackage? Well, the library can be used without the utilities - therefore the split. The utilities are end user utilities that use this library to convert icns to png and vice versa. > - [ BAD ] MUST: The sources used to build the package must match the > upstream source, as provided in the spec URL. Reviewers should use > md5sum for this task. If no upstream URL can be specified for this > package, please see the Source URL Guidelines for how to deal with this. > > md5sum = f6ab74c2cfd4ab5b11d1cb9114743c90 > > I can't actually download this from the given URL -- sourceforge gives > me a bunch of redirects and then a 404. I just followed the packaging guideline to provide a source URL that is on Sourceforge: https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net To check the md5, you can use a mirror. > - [ BAD ] MUST: Packages containing pkgconfig(.pc) files must > 'Requires: pkgconfig' (for directory ownership and usability). I'll fix this. (In reply to comment #2) > > - [ ??? ] MUST: The package must meet the Packaging Guidelines. > > Why -utils subpackage? > > Well, the library can be used without the utilities - therefore the split. The > utilities are end user utilities that use this library to convert icns to png > and vice versa. OK. > > - [ BAD ] MUST: The sources used to build the package must match the > > upstream source, as provided in the spec URL. Reviewers should use > > md5sum for this task. If no upstream URL can be specified for this > > package, please see the Source URL Guidelines for how to deal with this. > > > > md5sum = f6ab74c2cfd4ab5b11d1cb9114743c90 > > > > I can't actually download this from the given URL -- sourceforge gives > > me a bunch of redirects and then a 404. > > I just followed the packaging guideline to provide a source URL that is on > Sourceforge: > https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net > > To check the md5, you can use a mirror. The download.sf.net* urls just redirect to mirrors. The problem is none of the mirrors have the file. > > - [ BAD ] MUST: Packages containing pkgconfig(.pc) files must > > 'Requires: pkgconfig' (for directory ownership and usability). > > I'll fix this. OK. (In reply to comment #3) > The download.sf.net* urls just redirect to mirrors. The problem is none of the > mirrors have the file. Sorry. Thought of a different problems (sometimes the mirrors fail). The URL is not correct. I'll fix it. It must be: http://downloads.sourceforge.net/icns/%{name}-%{version}.tar.gz (In reply to comment #4) > (In reply to comment #3) > > The download.sf.net* urls just redirect to mirrors. The problem is none of the > > mirrors have the file. > > Sorry. Thought of a different problems (sometimes the mirrors fail). The URL is > not correct. I'll fix it. It must be: > > http://downloads.sourceforge.net/icns/%{name}-%{version}.tar.gz This is just further evidence that using %{name} can be dangerous :D. The md5sum of the file at that URL matches. I'm APPROVING this; you can fix the pkgconfig and Source0 stuff before you import it. And please, *before* :). (In reply to comment #5) > I'm APPROVING this; you can fix the pkgconfig and Source0 stuff before you > import it. And please, *before* :). Here they are: http://www.webalice.it/musuruan/RPMS/reviews/libicns.spec http://www.webalice.it/musuruan/RPMS/reviews/libicns-0.6.0-2.fc10.src.rpm Changelog: - Fixed Source0 URL - Added missing 'Requires: pkgconfig' to devel package Now I'll ask for CVS :) New Package CVS Request ======================= Package Name: libicns Short Description: Library for manipulating Macintosh icns files Owners: musuruan Branches: F-9 F-10 InitialCC: cvs done. libicns-0.6.0-2.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/libicns-0.6.0-2.fc10 libicns-0.6.0-2.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/libicns-0.6.0-2.fc9 Imported and built. libicns-0.6.1-1.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/libicns-0.6.1-1.fc10 libicns-0.6.1-1.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/libicns-0.6.1-1.fc9 libicns-0.6.1-1.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. libicns-0.6.1-1.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report. |