Fedora Merge Review: expat http://cvs.fedora.redhat.com/viewcvs/devel/expat/ Initial Owner: jorton
* RPM name is OK * Source expat-1.95.8.tar.gz is the same as upstream * Builds fine in mock * File list of expat looks OK * File list of expat-devel looks OK Needs work: * BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) (wiki: PackagingGuidelines#BuildRoot) * The %makeinstall macro should not be used (wiki: PackagingGuidelines#MakeInstall) * rpmlint of expat: W: expat summary-ended-with-dot A library for parsing XML. * rpmlint of expat-devel: W: expat-devel summary-ended-with-dot Libraries and include files to develop XML applications with expat. Minor: * Duplicate BuildRequires: autoconf (by automake), automake (by libtool) Notes: * Please use {?dist} in Release tag * You should Require(post) and Require(postun) ldconfig * Is it necessary to include static binaries (see the wiki: PackageGuidelines/Exclusion of Static Libraries
Thanks for the review. Fixed in -10: - Summary trailing dot removed - switch to preferred BuildRoot Won't fix: - %makeinstall is necessary for this non-DESTDIR-aware Makefile - dist tag use is not mandatory - explicit BuildRequires is a good thing - rpm automatically adds correct Requires for scriptlet interpreters - yes, static libraries are necessary for the static build of rpm
Hi Joe, > - %makeinstall is necessary for this non-DESTDIR-aware Makefile Agreed, from the guidelines: Fedora's RPM includes a %makeinstall macro but it must NOT be used when make install DESTDIR=% {buildroot} will work. So in this case it's not a blocker. > - dist tag use is not mandatory That's right > - explicit BuildRequires is a good thing True. > - rpm automatically adds correct Requires for scriptlet interpreters True as well. > - yes, static libraries are necessary for the static build of rpm There's no official policy on the inclusion of static libraries as far as I know, so this is no blocker either. This package is APPROVED.
%{?dist} is no must? Since when if true or did I misunderstand something?
Uh, look trought the list of MUST items in the Review Guidelines... nothing there. From http://fedoraproject.org/wiki/Packaging/DistTag: Do I Have To Use the Dist Tag? No. It is documented and standardized so that maintainers who wish to use it can do so, but it is not mandatory.
Assigning ticket to myself as per http://fedoraproject.org/wiki/PackageReviewProcess