Jeff King <peff@xxxxxxxx> writes: > One of our rules is that git-compat-util.h (or one of the well-known > headers which includes, cache.h or builtin.h) is included first in any > translation unit. Perhaps by now we can spell it out a bit more explicitly and update CodingGuidelines. We have: - 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. "such as" might be making things unnecessarily vague; I do not think a valid reason why we should say a .c file that includes "advice.h" as the first thing satisfies this requirement, for example. 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. it may be reasonable to say the first *.h file included must be one of git-compat-util.h, cache.h or builtin.h (and then we make sure that compat-util is the first thing included in either of the latter two). While I very much agree with the principle Robert alluded to [*1*], we may want to loosen that for .c files that include "builtin.h" or "cache.h" for the sake of brevity. For example, if you are builtin (hence you start by #include "builtin.h"), it may be reasonable to allow you to take whatever is in "cache.h" for granted [*2*]. So the rule might be: - The first #include in C files, except in platform specific compat/ implementations, must be either git-compat-util.h, cache.h or builtin.h. - 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. Optionally, - A C file must include only one of "git-compat-util.h", "cache.h" or "builtin.h"; e.g. if you include "builtin.h", do not include the other two, but it can consider what is availble in "cache.h" available to it. 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. [Footnote] *1* For example, even though "diff.h" may include "tree-walk.h" for its own use, a .c file that includes "diff.h" without including "tree-walk.h" that uses update_tree_entry() or anything that is declared in the latter is very iffy from semantic point of view. *2* Because that facility is so widely used inside the codebase, "builtin.h" includes "strbuf.h", so in addition to what are in "cache.h", we may want to allow builtin implementations to take strbufs for granted as well. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html