Re: [PATCH] Makefile: enable DEVELOPER by default

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 4, 2018, 06:09 UTC
Message-ID: <20180804060928.GB55869@aiede.svl.corp.google.com>
In-Reply-To: <20180804020255.225573-1-sbeller@google.com>

Hi,

Stefan Beller wrote:

Show 7 quoted lines

> Reviewer bandwidth is limited, so let's have the machine of the
> (potentially new) contributor warn about issues with the code by default.
>
> As setting DEVELOPER, the compiler is stricter and we may run into problems
> on some architectures. But packagers of said platforms are knowledgeable
> enough to turn off this flag. (Also they are fewer than the number of new
> contributors)

Which architectures would we run into problems on? Aren't those bugs that should themselves be fixed?

I think you are right that the packagers will cope with whatever setting we choose. My main concern is not about them but about other people building from source in order to run (instead of to develop) Git, and by extension, the people they go to for help when it doesn't work. I have lots of bitter experience of -Werror being a support headache and leading to bad workarounds when someone upgrades their compiler and the build starts failing due to a new warning it has introduced.

Show 14 quoted lines

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 41b93689add..95aa3ff3185 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -497,6 +497,8 @@ ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
>  ALL_LDFLAGS = $(LDFLAGS)
>  STRIP ?= strip
>  
> +DEVELOPER=1

I like the idea of making this more discoverable to new contributors. It seems that Documentation/SubmittingPatches doesn't mention this setting. Should it?

Should a non-DEVELOPER build print a note encouraging enabling this setting in case you're developing patches meant for submission to the project?

Should we have a CONTRIBUTING.md file suggesting this setting? Other ideas for ensuring it's enabled for those who need it?

Thanks and hope that helps, Jonathan