SPEC: http://v3.sk/~lkundrak/SPECS/pnglite.spec SRPM: http://v3.sk/~lkundrak/SRPMS/pnglite-0.1.17-1.el5.src.rpm Description: pnglite is a C library for loading PNG images. It was created as a substitute for libpng in situations when libpng is more than enough. It currently requires zlib for inflate and crc checking and it can read the most common types of PNG images. The library has a small and simple to use interface. Notes: RPMlint complains about no documentation files, but there are no documentation files in the distribution. See the comment in %files. There's a typo in Summary line, I'd fix that on import or next package spin: A lighteeight C library for loading PNG images
[16:37:47] < lkundrak> cassmodiah: I'm wondering if you can review pnglite? [16:37:57] < lkundrak> cassmodiah: should be fairly simple -- just one .c file :) [16:38:46] < cassmodiah> i'm not sure with this [16:38:58] < cassmodiah> wait, i know which is a better guy for that [16:39:00] < cassmodiah> rsc ping [16:39:14] < lkundrak> :) [16:44:20] < rsc> cassmodiah: pong [16:46:02] < cassmodiah> rsc do you have time for a short look at a review request? [16:47:05] < cassmodiah> https://bugzilla.redhat.com/show_bug.cgi?id=478581 [16:47:06] < buggbot> Bug 478581: medium, low, ---, nobody, NEW, Package Review: pnglite - A lightweight C library for loading PNG images [16:49:44] < rsc> lkundrak: I'm not absolutely sure, whether the ABI soname stuff is clever. Was this practised somewhere else was well? [16:51:17] < lkundrak> rsc: I'm not really sure. I know I did that, but don't remember if it was in Fedora package. But if upstream is not willing to maintain soname (which I did not ask though), is there anything else we can do? [16:53:16] < lkundrak> rsc: you know -- pnglite is just a single pnglite.c file, upstream doesn't even maintian a build script where soname could be enforced [16:54:52] < rsc> lkundrak: understood. Why is %{abi_minor} == %{release}? If you've to rebuild for e.g. a new GCC; soname would be minorly bumped... [16:55:31] < lkundrak> rsc: abi_minor was not meant to be the part of soname. If it is, then it is a mistake [16:56:04] < lkundrak> rsc: -Wl,--soname,libpnglite.so.%{abi_major} [16:56:10] < lkundrak> rsc: there's only abi_major [16:56:12] < rsc> libpnglite.so.%{abi_major}.%{abi_minor} [16:56:24] < lkundrak> rsc: right, that's not a soname. only a filename. [16:56:35] < rsc> ah right. Sorry. [16:57:43] < rsc> but anyway: If you bump %{version}, %{release} should be reset. Then you would have to increase %{abi_major} to ensure, that libpnglite.so.%{abi_major}.%{abi_minor} is newer, not older from the filename. [16:57:53] < rsc> is that expected behaviour? [17:00:18] < lkundrak> rsc: yes. that's why there's a huge comment in the beginning [17:00:54] < lkundrak> rsc: but in case a new major version is released without abi change, then we would unnecessarily change soname [17:01:05] < lkundrak> rsc: I don't think that's likely though [17:11:03] < lkundrak> rsc: on a second thought -- I don't know what is the last digit in the library file name for. Probably it won't be a problem if it was reset. Probably we could decouple if from release. [17:17:59] < cassmodiah> lkundrak one of my teeworlds testers has no sound [17:19:26] < lkundrak> cassmodiah: well, I do not think it is related to any change we did [17:20:34] < rsc> lkundrak: I'll try the formal review in a few minutes
Lubomir, can you please add the license to %doc of the base package? I know, it is in the header file of -devel as well, but that's not perfect IMHO. So just the license for %doc, not the whole header file. Maybe the license just from the header file as Source2 simply. Package seems sane to me, source matches, debuginfo is okay, description and overall readability of the spec file, file locations/ownings fit etc. Please add license to %doc and correct your typo in Summary. As this is very minor: APPROVED.
(In reply to comment #2) > Lubomir, can you please add the license to %doc of the base package? I know, > it is in the header file of -devel as well, but that's not perfect IMHO. So > just the license for %doc, not the whole header file. Maybe the license just > from the header file as Source2 simply. That can't be done, see [1]. And given the size of the package and license being included in the header I don't think it would make sense to make upstream distribute one more file with a copy of the license. [1] https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text New Package CVS Request ======================= Package Name: pnglite Short Description: A lightweight C library for loading PNG images Owners: lkundrak, cassmodiah Branches: EL-5 F-9 F-10
cvs done.
Imported and built. Thanks!