Pavel Roskin wrote:
Hello, Roland!

I have found that your changes that introduced MC_ARG_ENABLE_DEVELOPER_MODE cause configure script to ignore CFLAGS passed on the command line if --enable-developer-mode is not
specified. Also, it removes the optimization option "-O2" that is
added by configure.

Ok, I hadn't tested if the -O2 default option is still passed around. It
it obviously good to have that as default.

Also, I don't see a need for having a separate "developer mode". We shouldn't have warnings that only developers see. I believe that
-Wall is quite reasonable.  According to gcc documentation, -Wall
enables warnings that can be easily worked around.  I believe -W is
excessive because it enables warnings that are not so useful and that
cannot be worked around without making the code uglier.  We shouldn't
encourage developers to work around all such warnings.  I'm fine with
adding more specific warnings the maintainer-only scripts, such as

But we almost have reached a warning-free build with -W -Wall. There's
only a bit of Samba code and a few others.

Finally, optimization is even more useful for end users than for developers. Your patch does exactly the wrong thing - it kills the optimization flags for the end users and leaves them on for the developers.

Another mistake. I intended to write CFLAGS="-Wall ${CFLAGS}".

I believe that having one more configure option makes maintaining the
 code more difficult.  It creates more possibilities for bugs that
not everybody can see.

I have continued working on the viewer, and I have used the
MC_ENABLE_DEBUGGING_CODE flag to dump useful information into a file
when I press the 't' key. This is very helpful for me, but shouldn't be seen by an end user.

I'm also removing m4/ri-gcc-warnings.m4, which is now unused.

That's fine.

I'm replacing all that stuff with a simple piece of code:

dnl If default CFLAGS is used with gcc, add -Wall if test -z
"$ac_env_CFLAGS_set"; then if test -n "$GCC"; then CFLAGS="$CFLAGS
-Wall" fi fi

This looks good.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]