[Bug 576591] Review Request: iptraf-ng

bugzilla at redhat.com bugzilla at redhat.com
Tue Apr 6 18:39:58 UTC 2010


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

--- Comment #4 from Terje Røsten <terjeros at phys.ntnu.no> 2010-04-06 14:39:55 EDT ---
Thanks, good progress, some more comments.

Uou seem to be fan of %attr in %files, in general we only use that if
something is special or uncommon, in your case I see no such things.
Set bits correct in %files and let the %files section be simple.

I would prefer that you changed:

mkdir -p $RPM_BUILD_ROOT/etc/logrotate.d/
cp %{SOURCE1} $RPM_BUILD_ROOT/etc/logrotate.d/iptraf

mkdir -p $RPM_BUILD_ROOT/var/lock/iptraf
mkdir -p $RPM_BUILD_ROOT/var/log/iptraf
mkdir -p $RPM_BUILD_ROOT/var/lib/iptraf

to:

install -D -p -m 0644 %{SOURCE1}
$RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/iptraf-ng

# reduce to one line, preserve timestamps, correct perms. use proper macro and
change to iptraf-ng (the proper name)

and 

install -d -m 0755 $RPM_BUILD_ROOT%{_localstatedir}/{lock,log,lib}/iptraf-ng

# same thing here.

Now with %install fixed, these lines in %files:

%attr(755,root,root) %{_prefix}/bin/*
%{_mandir}/*/*
%dir %attr(755,root,root) /var/lock/iptraf
%dir %attr(755,root,root) /var/log/iptraf
%dir %attr(755,root,root) /var/lib/iptraf
%dir %attr(644,root,root) %config(noreplace) /etc/logrotate.d/iptraf

should be

%{_bindir}/iptraf-ng
%{_bindir}/rawtime
%{_bindir}/rvnamed
%{_mandir}/man8/iptraf.8*
%{_mandir}/man8/rvnamed.8*
%{_localstatedir}/lock/iptraf-ng
%{_localstatedir}/log/iptraf-ng
%{_localstatedir}/lib/iptraf-ng
%config(noreplace) %{_sysconfdir}/logrotate.d/iptraf-ng


BTW: the rawtime name is a bit on the generic side, a man page about
the tool would not hurt.

You should really decide on iptraf versus iptraf-ng.

Is this iptraf og iptraf-ng?

-- 
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 package-review mailing list