[Bug 576591] Review Request: iptraf-ng
bugzilla at redhat.com
bugzilla at redhat.com
Thu Apr 8 08:48:36 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 #5 from Nikola Pajkovsky <npajkovs at redhat.com> 2010-04-08 04:48:31 EDT ---
(In reply to comment #4)
> Thanks, good progress, some more comments.
>
> Uou seem to be fan of %attr in %files, in general we only use that if
:D I'm definitely not a fan of writing spec files and mainly rewriting spec.
> 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
>
Fixed.
> # 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
>
Fixed. I didn't know I can you {brum,brum}
> # 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
>
Fixed.
>
> 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?
This is iptraf-ng and I left the original naming. There is some *wild* header
called dirs.h. It works as config.h in autotools and in my TODO is get rid of
this machinery and use config.h. I'm mostly working on ABRT so I don't have
much time, but I feel in my bones that this weekend this should be done :)
I hope this is a last round :)
spec: http://npajkovs.fedorapeople.org/iptraf-ng.spec
srpm: http://npajkovs.fedorapeople.org/iptraf-ng-1.0.2-3.fc14.src.rpm
--
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