[Bug 465636] Review Request: FireHOL - a Linux iptables packet filtering firewall builder for humans

[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=465636





--- Comment #10 from Michael Schwendt <mschwendt at gmail.com>  2009-09-20 09:57:18 EDT ---
> Calm down big boy.

I second that.


> > A simple "Looks OK" is sloppy reviewing,

Not at all. This point is entirely moot. Whether it's a very brief "Looks OK"
or "APPROVED" or a huge list of MUST/SHOULD/OK/NOTOK/FAIL/BLAH items that
copies the entire ReviewGuidelines Wiki page, it doesn't matter. All that
matters is whether a reviewer has checked the package actually. The reviewer
takes responsibility for serious mistakes. But don't forget that there are two
people. The packager is the second one, who ought to review the package, too.
;)


> # Don't start the firewall automatically
> sed -i -e 's,# chkconfig: 2345 99 92,# chkconfig: - 20 80,' firehol.sh

That's sloppy. sed -i is worse than a clean patch file. If the source file
changes, your sed transformation no longer applies. Silently. On the contrary,
a patch would fail and terminate the build. Adding a guard after the sed
transformation is highly recommended.

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