Spec URL: http://www.alexhudson.com/fedora/iniparser/iniparser.spec SRPM URL: http://www.alexhudson.com/fedora/iniparser/iniparser-3.0b-1.fc10.src.rpm Description: IniParser is a simple ANSI-C library used by other applications to parse "ini-style" files as used mainly on Windows. There is a small problem with this library; it doesn't come with a "full" build system - it has a Makefile which compiles the library, but doesn't even have "make install", thus the manual fiddling around in the spec file. I suspect this makes the -debuginfo package less useful - it doesn't come with copies of the source, for example. Another issue is that the library is simply called libiniparser.so.0 - there is no minor version. I imagine this would make it difficult to correctly work out dependencies (for example, if new API was added to the library later and an application wished to declare that dependency) - however, there is also a compatibility issue there if it's changed just for Fedora? I'm contemplating writing a patch which would effectively drop in a more standard autotools build, but would appreciate any easier ideas. Also, this is only the second package I've submitted and I'm not a package maintainer (yet, hopefully ;)
Good: + Package name fits with nameing guidelines. + Source file could downloaded with spectool + Tar ball in package fits with upstreams (md5sum: 89b7d97b9fb24ce4c31743cc4d13ce44) + Package contains valid Group + Package contains valid license tag + License from the license tag is a valid OSS license + Consistently usage of rpm macros + Package contains varbatin copy of the license text + Rpmlint is quite on source rpm + Rpmlint is quite on binary rpm + Rpmlint is quite on intalled rpm + Rpmlint is quite on debuginfo package, Debuginfo package contains valid source files. + Package contains scriptlets for ldconfig + Package contains a devel subpackage + devel subpackage contains Requires on main package + Files contains no files belongs to other packages + Files have proper file perms + %doc stanza is small, so we need no separate doc subpackage + Package builds fine on koji, if you are add '-fPIC' + Package contains proper %changelog stanza + Local install of the package works fine + Local uninstall of the package works fine Bad: - Local build failed, Please add '-fPIC' compilerflag if you want to create a dynamic library - Please use install for installing the files of your package - Please notify upstream, that each source file should have a copyright notice - Please remove %check stanza, if you don't have the testsuite which should be executing during the build
Jochen, Thank you very much for taking the time out to review my package, much appreciated. I've updated here (SRPM has changed): Spec URL: http://www.alexhudson.com/fedora/iniparser/iniparser.spec SRPM URL: http://www.alexhudson.com/fedora/iniparser/iniparser-3.0b-2.fc10.src.rpm I've addressed the three points you've raised about the .spec, and will engage upstream on the copyright issue. I did a scratch build on Koji (task id #1054148), and can confirm it builds - sorry, I should have done that originally.
Good: + Local build works fine. + Files are install with the install programm. + Empty %check stanza is removed. Bad: - Package doesn't contains verbatin copy of the license text, but upstream tar ball contains a LICENSE file. If you are add the LICENSE file into the %doc stanza you package is APPROVED.
Hello, Jochen. Are you a sponsor? (sorry if you are so) Any NEEDSPONSOR blocking review tickets needs the approval by sponsor members.
Many thanks again, Jochen - I've added the LICENSE and bumped the rpm. Spec URL: http://www.alexhudson.com/fedora/iniparser/iniparser.spec SRPM URL: http://www.alexhudson.com/fedora/iniparser/iniparser-3.0b-3.fc10.src.rpm
Sorry, I'm not a sponsor.
Hey Alex. I would be happy to look at this and bug 479835. Look for more info later today.
On quick comment here: Is this a pre-release/post-release or snapshot package? If so, see: http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Non-Numeric_Version_in_Release Ie, is it a beta of an upcoming 3.0 release? If so the version should probibly be: 3.0-0.1.b
Hi Kevin, Thanks for taking a look at this! You're right about the version number - for some reason, I had assumed that this was a subsequent release to 3.0, but it appears not. I've updated the spec. to take into account the pre-release numbering: http://www.alexhudson.com/fedora/iniparser/iniparser.spec This is actually rpm-older than the previous version, but hopefully that's not really a problem at this point. Also, while this is "pre-release", iniparser seems to be developed quite slowly and conservatively, and I would expect most software that uses it would expect this more up-to-date version.
I don't see any further issues here, so this package is APPROVED. removing NEEDSPONSOR as I am sponsoring Alex. Thanks for reviewing this Jochen!
Alex: Care to request cvs here?
Yes I missing the CVS-Admin request to, so what is the state of this ticket?
New Package CVS Request ======================= Package Name: iniparser Short Description: a library for parsing ini-style files Owners: alexh Branches: F-10 F-11 InitialCC: Apologies for dropping the ball on this one; my dev. machine was out for a while and I'd forgotten it was outstanding - thanks for reminding me!
cvs done.
Imported and built, thanks.
Package Change Request ====================== Package Name: iniparser New Branches: el6 Owners: jcapik jmarrero