Re: [PATCH v2] add DEVELOPER makefile knob to check for acknowledged warnings

[Michael Haggerty <mhagger@alum.mit.edu>]

On 02/25/2016 09:42 AM, larsxschneider@gmail.com wrote:
> From: Lars Schneider <larsxschneider@gmail.com>
> 
> We assume Git developers have a reasonably modern compiler and recommend
> them to enable the DEVELOPER makefile knob to ensure their patches are
> clear of all compiler warnings the Git core project cares about.
> 
> Enable the DEVELOPER makefile knob in the Travis-CI build.
> 
> Suggested-by: Jeff King <peff@peff.net>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
> 
> This patch is the successor of "[PATCH v1] travis-ci: override CFLAGS properly,
> add -Wdeclaration-after-statement" [1] which enables compiler warnings for the
> Travis-CI build.
> 
> Peff suggested to codify the knowledge about the compiler warnings the Git
> project cares about [2] which I have done here.
> 
> The only problem is the "-Wold-style-declaration" compiler warning as this is
> only supported by GCC and not Clang. Should we ignore that warning or would you
> prefer to detect the GCC compiler and add the warning? The Linux kernel project
> does a similar thing [3].
> 
> 
> Thanks,
> Lars
> 
> 
> [1] http://thread.gmane.org/gmane.comp.version-control.git/285752
> [2] http://article.gmane.org/gmane.comp.version-control.git/285761
> [3] https://github.com/torvalds/linux/blob/6dc390ad61ac8dfca5fa9b0823981fb6f7ec17a0/Makefile#L303-L306
> 
> 
>  .travis.yml                    |  2 +-
>  Documentation/CodingGuidelines |  4 ++++
>  Makefile                       | 12 ++++++++++++
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> [...]
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index c6e536f..1c676c2 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -171,6 +171,10 @@ For C programs:
> 
>   - We try to keep to at most 80 characters per line.
> 
> + - As a Git developer we assume you have a reasonably modern compiler
> +   and we recommend you to enable the DEVELOPER makefile knob to
> +   ensure your patch is clear of all compiler warnings we care about.
> +

Instead of saying "enable the DEVELOPER makefile knob" could you be more
explicit? Like maybe "create a file called `config.mak` and add the line
`DEVELOPER=1` to it"? (Or whatever is your preferred way to tweak this
setting.)

>   - We try to support a wide range of C compilers to compile Git with,
>     including old ones. That means that you should not use C99
>     initializers, even if a lot of compilers grok it.
> [...]

Michael