From: Christian Couder <chriscool@tuxfamily.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
Nathaniel P Dawson <nathaniel.dawson@gmail.com>,
Johannes Sixt <j.sixt@viscovery.net>,
Nanako Shiraishi <nanako3@lavabit.com>
Subject: [PATCH] Documentation/CodingGuidelines: improve header includes rules
Date: Tue, 14 Apr 2009 00:34:33 +0200 [thread overview]
Message-ID: <20090414003433.39cbdea2.chriscool@tuxfamily.org> (raw)
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/CodingGuidelines | 26 ++++++++++++++++++++++----
1 files changed, 22 insertions(+), 4 deletions(-)
Sorry for the delay but I have been busy with other things.
The goal of these rule is to codify the current practice and
to keep things simple to use for the developer while keeping
some sanity.
So there are 4 rules and I hope it's enough:
- The first one was already in the document but is strenghtened
a bit.
- The second one existed informaly.
- The third one means that for example if we have "revision.h"
that includes "diff.h" and "commit.h", then it's ok to include
"revision.h" in a C file, only if at least one feature from
"revision.h" is actually used in the C file.
It is not ok to include "revision.h" if features from "diff.h"
and "commit.h" are used but no feature from "revision.h" is
used.
But on the other hand if features from bith "revision.h" and
"diff.h" are used in a C file, then the rule does not state that
"diff.h" has to be included if "revision.h" is included. So in
practice this means that it would be ok to remove an include of
"diff.h" if "revision.h" is included.
- The fourth and last rule tries to keep things simple to use
for the developper and quite sane. Here are some part of an email
I already sent. This will hopefully explain why I think this rule
is needed:
> Well, I wanted to say that for a header file frotz.h in the
> project:
>
> $ cat >1.c <<\EOF
> #include "cache.h"
> #include "frotz.h"
> EOF
> $ cc -Wall -c 1.c
>
> should not fail.
>
> (Ok, in practice, let's say that something like:
>
> $ cc -Wall -DSHA1_HEADER='<openssl/sha.h>' -c 1.c
>
> should not fail.)
>
> I think [the fact that the above fails] may become a problem if
> a header needs more headers to be included before it, and those
> latter headers again need more headers and so on.
>
> The advantage of having and using "cache.h" is that things are
> quite simple.
> You include cache.h and then a few more headers for special
> features you need, and, here you go, you can code up something
> quite interesting without worrying too much about includes.
> (Though the compilation time is perhaps a little longer than it
> could be.)
>
> If we loose this simplicity because we don't take care or for
> some theoretical reason, then I think we have lost everything
> that really matters.
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index b8bf618..181cb2d 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -115,10 +115,6 @@ 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.
-
- If you are planning a new command, consider writing it in shell
or perl first, so that changes in semantics can be easily
changed and discussed. Many git commands started out like
@@ -132,3 +128,25 @@ For C programs:
- When we pass <string, length> pair to functions, we should try to
pass them in that order.
+
+About header file includes:
+
+ - The first #include in C files, except in platform specific
+ "compat/" implementations, should be "git-compat-util.h" or
+ "cache.h" or "builtin.h".
+
+ - System header files should be included in "git-compat-util.h",
+ except for specific system headers like "syslog.h" in "daemon.c".
+ (This is because for portability reasons we want the compiler to
+ always see the same system headers in the same order.)
+
+ - After the first #include in a C file, only header files containing
+ features that are actually used in the C file should be included.
+ (This means that it is not ok to include an header file only
+ because this header file includes other header files with features
+ that are used in the C file.)
+
+ - Git header files, when they are included in a C file after
+ "cache.h", should not require any other header file to be included
+ before them for the C file to compile cleanly. (This is because we
+ want to keep it simple to use features defined in our headers.)
--
1.6.2.2.572.g4420a
next reply other threads:[~2009-04-13 22:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-04-13 22:34 Christian Couder [this message]
2009-04-14 16:24 ` [PATCH] Documentation/CodingGuidelines: improve header includes rules Johannes Schindelin
2009-04-15 7:58 ` Jeff King
2009-04-16 5:10 ` Christian Couder
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=20090414003433.39cbdea2.chriscool@tuxfamily.org \
--to=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j.sixt@viscovery.net \
--cc=nanako3@lavabit.com \
--cc=nathaniel.dawson@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).