[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


Orcan Ogetbil <orcanbahri at yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |orcanbahri at yahoo.com




--- Comment #1 from Orcan Ogetbil <orcanbahri at yahoo.com>  2008-10-06 20:32:13 EDT ---
Hi, I just went through your package. It needs some work. Here are my notes:
-------------------------------------------------------------------------
The summary needs to be beefed up and more informative IMHO.
-------------------------------------------------------------------------
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).
---------------------------------------------------------------
In the %files section, we prefer to use 
   %defattr(-, root, root, -)
Also
   %attr(0644, root, root)
are redundant. Instead you should use
   install -pm 644 pyroman/* $RPM_BUILD_ROOT/%{python_sitelib}/%{name}/
and
   install -Dpm 644 doc/pyroman.8 $RPM_BUILD_ROOT/%{_mandir}/man8/pyroman.8
in the %install section. Also make sure you use the switch -p of the "install"
command to preserve the timestamps whenever it makes sense (usually for the
non-compiled files).
-------------------------------------------------------------------------
License is MIT, not GPL
-------------------------------------------------------------------------
   BuildRequires: python-devel
is redundant. Package will build without it.
   Requires:  python
is not necessary either. rpmbuild will pick that up.
-------------------------------------------------------------------------
You can further replace
   dir %{python_sitelib}/%{name}/
   %{python_sitelib}/%{name}/*.py*
with 
   %{python_sitelib}/%{name}
to improve the spec file.
-------------------------------------------------------------------------
Make sure you make use of %{name} and %{version} macros in addition to the
predefined directory macros.
For instance, you use
   install -D bin/pyroman $RPM_BUILD_ROOT/usr/sbin/pyroman
in one line but
   %{_sbindir}/%{name}
on the other, which is inconsistent and not desired.

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