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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ramkumar Ramachandra <artagnon@xxxxxxxxx> 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.
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]