Hide Forgot
Fedora Merge Review: iso-codes http://cvs.fedora.redhat.com/viewcvs/devel/iso-codes/ Initial Owner: dsantani@redhat.com
Partial review: MUSTFIX: * %{_datadir}/locale/*/LC_MESSAGES/*.mo is used instead of find_lang. See: http://fedoraproject.org/wiki/Packaging/Guidelines#locales * Add dependency on pkgconfig to -devel package. * Don't use %makeinstall. See: http://fedoraproject.org/wiki/Packaging/Guidelines#MakeInstall * Either own /usr/share/xml, or depend on xml-common. * The ISO standards named in description should be changed to their proper name, "ISO 639" (instead of "ISO-639"), "ISO 3166-1" (instead of "ISO-3166"), and "ISO 3166-2" (instead of "ISO-3166-2"). SUGGESTIONS: * Consider using new upstream version (1.0a is available from: http://ftp.debian.org/debian/pool/main/i/iso-codes/) * No need to use %{__mv} and %{__rm}. Use normal mv and rm. * Use "make %{?_smp_mflags}" * Add a dot at the end of -devel's description. * Could do %{_datadir}/iso-codes Instead of: %dir %{_datadir}/iso-codes %{_datadir}/iso-codes/*.tab
All the material objections have been taken care of.
Stalled review, needs a new reviewer to give the rubber stamp.
Created attachment 317286 [details] Update to 3.3, address remaining items Here's a patch that updates to 3.3, addresses remaining comments and removes the unneeded debug_package override.
Why is the debug_package override unnecessary ?
It's a no-op for noarch packages, see "rpm -E %debug_package".
Ah, ok. If you want to go for accuracy in the description, I'd propose to remove the reference to "po format", since the .po files are only in the source package, the binary package contains mo files. The patch looks fine to me, please go ahead and apply it and rebuild.
Created attachment 317375 [details] Take 2 I tried to, but was denied by CVS ACLs: **** Access denied: scop is not in ACL for rpms/iso-codes/devel The 3.3 tarball has been uploaded and the complete patch with fine tuned description is attached.
I guess you need to ask parag to open it up, he seems to be the maintainer...
I don't particularly need commit access to this package at the moment, and opening it up for the "uberpackager" group will happen when it'll happen (https://www.redhat.com/archives/fedora-announce-list/2008-August/msg00007.html), but meanwhile if someone can commit the patch, we could get this merge review out of the list of open ones...
Ville, I have opened ACLs now. Feel free to commit your patch as I am unable to download new source release 3.3. I see ftp release directory contains no release tarballs now. looks like upstream ftp maintainance issue to me.
Thanks, done and built, closing as approved: http://koji.fedoraproject.org/koji/taskinfo?taskID=838912 I've had some trouble getting a file listing from the upstream FTP site as well but it worked for me a few minutes ago with Firefox. I've never succeeded to download the tarball with wget but got it every time with curl (could be related to file listing issues, dunno).
Package Change Request ====================== Package Name: iso-codes New Branches: F-8 F-9 F-10 devel Owners: caillon pnemade
cvs admin done (Chris said he would like to be recognised as the formal package owner and have Parag as comaintainer. BTW there are already a lot of comaintainers in the desktop group - dunno if it is really necessary to list them all...?)
This bug is still assigned to a review of iso-codes-3.12-1, thus I'm using it: All seems fine except it's still using %{__rm}. Might not be a problem, but would be nice to have it replaced as mentioned above.
Just do it...that kind of cleanup is not worth wasting bugzilla comments on...