[Bug 463922] Review Request: ifstat - Interface statistics

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





--- Comment #12 from Michael Schwendt <bugs.michael at gmx.net>  2008-10-16 19:04:07 EDT ---
* I don't see anything wrong with the suggested iconv line.

Hint: Run iconv on all files that need to be converted, then
create a diff to replace your patch.

If you disagree with the reviewer, don't jump to another
solution, but find out whether you really tried the same fix.


* --enable-debug in general is the wrong thing to do, especially if
it sets more than just compiler flags like -g. This program also sets
-DDEBUG which bears the risk of enabling and compiling optional code
that is not wanted. This particular program doesn't evaluate the DEBUG
define yet in its own source files, but other programs use such flags
to toggle assertions/debugging output and diagnostic code paths.

What you want instead is to simply ensure that the Fedora CFLAGS are
accepted by the Makefile and are not modified in unwanted ways.
Read the build log.


* Re: comment 4

> %{__make} install DESTDIR=$RPM_BUILD_ROOT CFLAGS="$RPM_OPT_FLAGS"
> INSTALL="install -p" 

Correct except for the CFLAGS definition, which must NOT be present
here. Nothing during "make install" is supposed to compile anything.

Also note that the %makeinstall macro may be used where a patch
like ifstat-destdir.patch were far bigger. That macro is a little
bit dangerous in some cases, so prefer the recommended make install
invocation.


* Licence is "GPLv2+" not "GPLv2". See header of the .c/.h files.
Note the "or [...] any later version".

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