Re: [PATCH] Makefile: enable DEVELOPER by default

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Stefan Beller wrote:

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

> 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