Bug 479951
| Summary: | Review Request: iniparser - a library for parsing ini-style files | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Alex Hudson (Fedora Address) <fedora> |
| Component: | Package Review | Assignee: | Kevin Fenzi <kevin> |
| Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | low | ||
| Version: | rawhide | CC: | fedora-package-review, jcapik, jmarrero, jochen, notting |
| Target Milestone: | --- | Flags: | kevin:
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: | 2009-05-24 11:12:48 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: | |||
|
Description
Alex Hudson (Fedora Address)
2009-01-14 09:24:41 UTC
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 |