Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=731898 --- Comment #4 from Matt Domsch <matt_domsch@xxxxxxxx> 2011-08-21 23:58:37 EDT --- This is not a thorough review, but some things that jumped out at me reading the spec file. * Drop the "All rights reserved." text. If you're asserting separate copyright license for the spec file itself (which I would prefer not) then explicitly state the license here. Presumably you mean for the spec file to be licensed under the same license as the rest of the package (BSD), so make that explicit. * Drop the Prefix: line * What is OSshort ? On Fedora 14, that resolves to nil; where is it coming from? * Source0 and 1 need URLs to the upstream tarballs * There are better forms for BuildRoot (though it's ignored on F15+) * I find it unusual for %changelog to come so high in the file; I'm used to it being at the end. I guess it works where it is though. * libtool in the oldest distro noted here (EL5) is v2.2, so I would be surprised if you need your own copy ever, much less the ifarch x86_64 test and the other tests around it during %build. * Don't use %makeinstall if it can be avoided (and it can always be avoided with upstream patching). http://fedoraproject.org/wiki/Packaging:Guidelines#Why_the_.25makeinstall_macro_should_not_be_used * you're massive echoed config file works because your config doesn't happen to have single quotes in it. Better to use a shell here document, e.g. cat > opendkim.conf << EOF big long config file EOF * %post creates /var/run/opendkim and /var/spool/opendkim and /etc/opendkim but they are not owned by the package in %files. Better to create those in %install with the desired permissions, and own them in %files with permissions as necessary. * consider creating the opendkim group first, then creating the user with that group, rather than fixing it up afterwards. Users and groups should be created in %pre, rather than %post, so that they are available to the system when file permissions are set in %files. e.g.: %pre getent group mirrormanager >/dev/null || groupadd -r mirrormanager getent passwd mirrormanager >/dev/null || \ useradd -r -g mirrormanager -d /var/lib/mirrormanager -s /sbin/nologin \ -c "MirrorManager" mirrormanager exit 0 %files %defattr(-,root,root,-) %{_datadir}/%{name} %attr(-,mirrormanager,mirrormanager) %dir %{_localstatedir}/lib/%{name}/ * consider moving key generation to startup of the service, rather than during RPM install time. This is what openssh does, for example. It's quite possible that there is insufficient entropy available at RPM install time to generate keys, which will then cause the install to hang. * -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review