https://bugzilla.redhat.com/show_bug.cgi?id=921797 --- Comment #29 from Adam Williamson <awilliam@xxxxxxxxxx> --- I'm a packager, so I can review this formally. Mostly I endorse what Trever has done so far in his unofficial review. Additional notes: 1. Is the dependency on postfix really necessary? The script can be called on its own, and it's at least conceivable that something other than postfix could call it to do something useful. I'm not sure the postfix dep achieves anything sane. 2. The definition of python_sitelib is unnecessary and unwanted unless you intend to build for EL-5. If you do intend to build for EL-5, you should at least conditionalize it. https://fedoraproject.org/wiki/Packaging:Python#Macros has a snippet you can copy/paste. 3. I think the guidelines are encouraging explicit specification of python2-devel or python3-devel, rather than python-devel. 4. You include the comment line "# Remove CFLAGS=... for noarch packages (unneeded)" in the %build section, but then include the CFLAGS= definition? This is a noarch package. It seems like you should drop that comment line, and drop the "CFLAGS="$RPM_OPT_FLAGS"" from the next line. 5. I'm not sure if it's valid to mark an *entire directory* as %config(noreplace) . I don't think I've seen that before. I think: %dir %{_sysconfdir}/python-policyd-spf %config(noreplace) %{_sysconfdir}/python-policyd-spf/policyd-spf.conf would be more conventional. I'm not actually sure what the effects of marking a directory as noreplace would be, and Google isn't immediately helpful. Aside from those notes, the package looks good to me. If you can resolve the above I'll mark it as approved. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=SQpg1wkhgw&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review