Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: postgrey https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=182027 ------- Additional Comments From mfleming+rpm@xxxxxxxxxxxxxxxx 2006-02-19 08:12 EST ------- NEEDSWORK Review for release 1: * RPM name is OK * Source postgrey-1.24.tar.gz is the same as upstream * This is the latest version * Builds fine in mock (FC4) * File list of postgrey looks OK * Hasn't made my test instance explode. Needs work: * BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) (wiki: PackagingGuidelines#BuildRoot) * Spec file: some paths are not replaced with RPM macros (wiki: QAChecklist item 7) - Noticed some uses of %{_var} vs. %{_localstatedir} in there.. * rpmlint of postgrey: rpmlint of postgrey-1.24-1.noarch.rpm E: postgrey zero-length /etc/postgrey/postgrey_whitelist_clients.local - %ghost this and make the user add it themselves once they've RTFM, IMO E: postgrey non-standard-gid /var/run/postgrey postfix E: postgrey non-standard-dir-perm /var/run/postgrey 0750 E: postgrey non-standard-uid /var/lib/postgrey postgrey E: postgrey non-standard-gid /var/lib/postgrey postgrey E: postgrey non-standard-dir-perm /var/lib/postgrey 0750 - These can be ignored IMHO W: postgrey dangerous-command-in-%post chown - Do it via %attr instead if you have to, I get the jitters when I see this. W: postgrey service-default-enabled /etc/rc.d/init.d/postgrey - Please don't give explosives to newbies. :-) * The package should contain the text of the license (wiki: Packaging/ReviewGuidelines) - Yank a copy from an appropriate site, even as a SourceX if you want, Conversely, ask upstream to put it in the tarball. * Missing dependancy on service for %postun (package initscripts) * Missing dependancy on service for %preun (package initscripts) * Missing dependancy on chkconfig for %post (package chkconfig) * Missing dependancy on chkconfig for %preun (package chkconfig) - The Requires and pre/post install scriptlets need fixing. - Drop the PreReq entirely. Is this a holdover from MDK or something else? - Requires: postfix >= 2.1.0 (no policy server capability in older versions AFAIK. Again, no explosives for the newbs.) - For Requires(%post) etc, use the defaults from the Services part of http://www.fedoraproject.org/wiki/ScriptletSnippets Notes: Other blockers: - Don't use %define for Name/Version/Release, use them directly. It also makes the spec little cleaner. If possible avoid %defines at all unless you really need them. - Use install rather than mkdir/cp for moving files around and creating your extra dirs. "Would be nice if..." - Any reason to move the defaults around? - upstream likes it's configs and spool in /etc/postfix & $postfix_spooldir respectively. - "Starting/Stopping $prog" in the init file looks a little ugly, when compared to other init files (not that most sysadmins *care*, but it stuck out for me :-)) Perhaps a simple "Starting/Stopping Postgrey:" would suffice? - Toss a %{?dist} in Release as good practice and to make distro upgrades simpler and cleaner. Stuff I liked: - Generated manpages. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. -- fedora-extras-list mailing list fedora-extras-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/fedora-extras-list
