Bug 476357 - Review Request: libicns - Library for manipulating Macintosh icns files
Review Request: libicns - Library for manipulating Macintosh icns files
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Conrad Meyer
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-12-13 09:07 EST by Andrea Musuruane
Modified: 2009-01-07 04:32 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-12-21 09:57:29 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
konrad: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
[Review] (8.00 KB, text/plain)
2008-12-17 19:49 EST, Conrad Meyer
no flags Details

  None (edit)
Description Andrea Musuruane 2008-12-13 09:07:10 EST
Spec URL: http://www.webalice.it/musuruan/RPMS/reviews/libicns.spec
SRPM URL: http://www.webalice.it/musuruan/RPMS/reviews/libicns-0.6.0-1.fc10.src.rpm
Description: libicns is a library providing functionality for easily reading and 
writing Macintosh icns files
Comment 1 Conrad Meyer 2008-12-17 19:49:38 EST
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.)
Comment 2 Andrea Musuruane 2008-12-18 06:14:46 EST
> - [ ??? ] 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.
Comment 3 Conrad Meyer 2008-12-18 06:35:46 EST
(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.
Comment 4 Andrea Musuruane 2008-12-18 06:45:45 EST
(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
Comment 5 Conrad Meyer 2008-12-18 07:09:35 EST
(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* :).
Comment 6 Andrea Musuruane 2008-12-20 08:13:28 EST
(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 :)
Comment 7 Andrea Musuruane 2008-12-20 08:20:44 EST
New Package CVS Request
=======================
Package Name: libicns
Short Description: Library for manipulating Macintosh icns files
Owners: musuruan
Branches: F-9 F-10
InitialCC:
Comment 8 Kevin Fenzi 2008-12-20 23:09:10 EST
cvs done.
Comment 9 Fedora Update System 2008-12-21 09:48:46 EST
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
Comment 10 Fedora Update System 2008-12-21 09:55:49 EST
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
Comment 11 Andrea Musuruane 2008-12-21 09:57:29 EST
Imported and built.
Comment 12 Fedora Update System 2009-01-05 12:48:53 EST
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
Comment 13 Fedora Update System 2009-01-05 13:09:37 EST
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
Comment 14 Fedora Update System 2009-01-07 04:16:11 EST
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.
Comment 15 Fedora Update System 2009-01-07 04:32:28 EST
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.

Note You need to log in before you can comment on or make changes to this bug.