Re: [PATCH 1/5] sequencer: factor code out of revert builtin

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 13, 2011, 23:10 UTC
Message-ID: <7vvcqnmxeu.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CALkWK0n7v15n_s3CNq1Qu3LHjYkV-ENAkv2b+oB+VBkyV+Sphw@mail.gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

>> Why did sequencer.h move to after dir.h?
>
> 1. I like the convention of including the "foo.h" as the last header
> in "foo.c".

I do not think it is a good convention. The implementation of "foo.c" may need to include many other headers for its own use of other APIs, and the declarations in "foo.h" may depend on some types declared in some of them, but by definition the latter is a subset of the former. Having "foo.h" at the end of "foo.c" makes it difficult for others to tell between the two.

A user of foo.h API should need to include only git-compat-util.h and foo.h to be able to use foo.h API in the ideal world, even though it may need to include other headers to use other APIs defined in them.

A workable alternative in a world that is not so perfect for a user of "foo.h" API is to include git-compat-util.h and what "foo.h" needs before including "foo.h" and then other headers it needs. I think the current source code takes this approach.

With that observation, it would probably make more sense if "foo.c" included the headers in the following order:

 - git-compat-util.h (or the prominent ones like "cache.h" that is known
   to include it at the beginning);
 - Anything the declarations in "foo.h" depends on;
 - "foo.h" itself; and finally
 - Other headers that "foo.c" implementation needs.

That way, people who want to use "foo.h" can guess what needs to be included before using "foo.h" a lot more easily.