[Bug 463035] Review Request: pyroman - Very fast firewall configuration tool

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





--- Comment #11 from Orcan 'oget' Ogetbil <oget.fedora at gmail.com>  2009-04-04 01:30:36 EDT ---
Phew, it's been 6 months. This was one of my first reviews and I barely
remember it.

Anyway, since then, there were some changes in the guidelines too. I had a
quick look at the package:

* The problem that I reported before remains:
  When I run the "pyroman" script I get the error:
      No rule files found in directory './examples/base'!
  I think you need to change the default_rules_path in the launching script (to
  %{_sysconfdir}/%name via 'sed', for instance).

* Please make use of the %{name} macro in %install

* Please use the -p switch of install extensively, to preserve timestamps.

* %setup -q -n %{name}-%{version}
is not necessary. 
  %setup -q
should be enough

* We certainly don't want
  %define _unpackaged_files_terminate_build 0

* The new guidelines suggest that, In the first line, %define should be
replaced by %global

? Where does SOURCE1 come from? Why the name fedora.init ?

! You probably don't need
  cp -p %SOURCE1 .
because you can use
  install -pm 0755 %{SOURCE1} %{buildroot}/%{_initrddir}/%{name}
in the %install section.

* Also there are two rpmlints that deserve attention:
   pyroman.noarch: W: service-default-enabled /etc/rc.d/init.d/pyroman
   pyroman.noarch: W: incoherent-subsys /etc/rc.d/init.d/pyroman $prog

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