Re: MC_ARG_ENABLE_DEVELOPER_MODE

[Roland Illig <roland illig gmx de>]

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
> maint/mctest.

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.

Roland