[Bug 532402] Review Request: APF - Advanced Policy Firewall

[bugzilla at redhat.com]

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=532402





--- Comment #3 from Scott Collier <boodle11 at gmail.com>  2009-11-20 19:02:26 EDT ---
No problem.  Just a couple more things here.  Please make the changes and I'll
have another look.

1. Now it does build.  rpmlint has issues with the rpm:

$ rpmlint ../RPMS/noarch/apf-9.7.1-2.noarch.rpm
apf.noarch: E: non-readable /etc/apf/allow_hosts.rules 0640
apf.noarch: E: non-readable /etc/apf/internals/multicast.networks 0640
apf.noarch: W: non-conffile-in-etc /etc/apf/internals/multicast.networks
apf.noarch: E: non-readable /etc/apf/apf 0750
apf.noarch: E: non-standard-executable-perm /etc/apf/apf 0750
apf.noarch: E: non-readable /etc/apf/internals/private.networks 0640
apf.noarch: W: non-conffile-in-etc /etc/apf/internals/private.networks
apf.noarch: E: non-readable /etc/apf/VERSION 0640
apf.noarch: W: non-conffile-in-etc /etc/apf/VERSION
<snip>
1 packages and 0 specfiles checked; 55 errors, 21 warnings.

You can get information on these errors / warnings here:

https://fedoraproject.org/wiki/Common_Rpmlint_issues

You can also get more verbose info with "rpmlint -i"

2. I'd take out the 

BuildArch:      noarch

tag.

3. add the %{?dist} tag to release.

4. Please don't chkconfig a service on by default:
chkconfig --level 345 apf on

See 

https://fedoraproject.org/wiki/Packaging:SysVInitScript#Why_don.27t_we....

5. instead of defining basedir:

%define         basedir /etc/apf

you can use the %{_sysconfdir} tag.

which would change:

find %{buildroot}%{basedir}/ -type f -exec chmod 640 {} \;
to:
find %{buildroot}%{_sysconfdir}/apf -type f -exec chmod 640 {} \;

and a few other places as well...

6. You can take out these two lines:

mkdir -p %{buildroot}/%{_docdir}/%{name}
cp -pf COPYING.GPL CHANGELOG README.apf %{buildroot}/%{_docdir}/%{name}/

and use the %doc tag (something like):
%doc COPYING.GPL CHANGELOG README.apf

so, in your %files, 
%doc %{_docdir}/apf/CHANGELOG

take out the %{_docdir}/apf/

-- 
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.