From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: Robert Schiele <rschiele@gmail•com>,
Zoltan Klinger <zoltan.klinger@gmail•com>,
GIT Mailing-list <git@vger•kernel.org>
Subject: Re: Unused #include statements
Date: Thu, 15 Jan 2015 15:20:09 -0800 [thread overview]
Message-ID: <xmqqy4p34onq.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20150115223836.GC19021@peff.net> (Jeff King's message of "Thu, 15 Jan 2015 17:38:37 -0500")
Jeff King <peff@peff•net> writes:
> On Thu, Jan 15, 2015 at 10:50:45AM -0800, Junio C Hamano wrote:
>> ...
>> Thoughts? I am not looking forward to a torrent of patches whose
>> sole purpose is to make the existing C files conform to any such
>> rule, though. Clean-up patches that trickle in at a low rate is
>> tolerable, but a torrent is too distracting.
>
> I don't think the "optionally" one above is that necessary. Not because
> I don't agree with it, but because I do not know that we want to get
> into the business of laying out every minute detail and implication.
> The CodingGuidelines document is meant to be guidelines, and I do not
> want to see arguments like "well, the guidelines do not explicitly
> _disallow_ this, so you must accept it or add something to the
> guideline". That is a waste of everybody's time.
Totally. I know we do not want to get into that business.
> A general philosophy + good taste (from the submitter and the
> maintainer) should ideally be enough.
Yes, "ideally" ;-)
> Which isn't to say we shouldn't clarify the document when need be. But I
> think what I quoted at the top already is probably a good improvement
> over what is there.
OK, thanks. Let's queue something like this for post 2.3 cycle,
then.
-- >8 --
Subject: CodingGuidelines: clarify C #include rules
Even though "advice.h" includes "git-compat-util.h", it is not
sensible to have it as the first #include and indirectly satisify
the "You must give git-compat-util.h a clean environment to set up
feature test macros before including any of the system headers are
included", which is the real requirement.
Because:
- A command that interacts with the object store, config subsystem,
the index, or the working tree cannot do anything without using
what is declared in "cache.h";
- A built-in command must be declared in "builtin.h", so anything
in builtin/*.c must include it;
- These two headers both include "git-compat-util.h" as the first
thing; and
- Almost all our *.c files (outside compat/ and borrowed files in
xdiff/) need some Git-ness from "cache.h" to do something
Git-ish.
let's explicitly specify that one of these three header files must
be the first thing that is included.
Any of our *.c file should include the header file that directly
declares what it uses, instead of relying on the fact that some *.h
file it includes happens to include another *.h file that declares
the necessary function or type. Spell it out as another guideline
item.
Helped-by: Jeff King <peff@peff•net>
Signed-off-by: Junio C Hamano <gitster@pobox•com>
---
Documentation/CodingGuidelines | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 894546d..578d07c 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -328,9 +328,14 @@ For C programs:
- When you come up with an API, document it.
- - The first #include in C files, except in platform specific
- compat/ implementations, should be git-compat-util.h or another
- header file that includes it, such as cache.h or builtin.h.
+ - The first #include in C files, except in platform specific compat/
+ implementations, must be either "git-compat-util.h", "cache.h" or
+ "builtin.h". You do not have to include more than one of these.
+
+ - A C file must directly include the header files that declare the
+ functions and the types it uses, except for the functions and types
+ that are made available to it by including one of the header files
+ it must include by the previous rule.
- If you are planning a new command, consider writing it in shell
or perl first, so that changes in semantics can be easily
next prev parent reply other threads:[~2015-01-15 23:20 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-15 3:43 Unused #include statements Zoltan Klinger
2015-01-15 4:14 ` Robert Schiele
2015-01-15 6:33 ` Jeff King
2015-01-15 18:50 ` Junio C Hamano
2015-01-15 22:38 ` Jeff King
2015-01-15 23:20 ` Junio C Hamano [this message]
2015-01-16 0:00 ` Jeff King
2015-01-20 2:08 ` Zoltan Klinger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqy4p34onq.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=peff@peff$(echo .)net \
--cc=rschiele@gmail$(echo .)com \
--cc=zoltan.klinger@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox