Re: [PATCH v2] add DEVELOPER makefile knob to check for acknowledged warnings
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Feb 25, 2016, 10:20 UTC
- Message-ID
- <56CED56D.5040202@alum.mit.edu>
- In-Reply-To
- <1456389742-48052-1-git-send-email-larsxschneider@gmail.com>
On 02/25/2016 09:42 AM, larsxschneider@gmail.com wrote:
Show 52 quoted lines
> 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