[Bug 463035] Review Request: pyroman - Very fast firewall configuration tool
bugzilla at redhat.com
bugzilla at redhat.com
Tue Oct 7 00:32:14 UTC 2008
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.
More information about the Fedora-package-review
mailing list