git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: David Aguilar <davvid@gmail.com>, git@vger.kernel.org
Subject: Re: [RFC PATCH 2/2] headers: include dependent headers
Date: Mon, 08 Sep 2014 11:05:50 -0700	[thread overview]
Message-ID: <xmqqbnqqng1t.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <540B7AA3.1090201@web.de> ("René Scharfe"'s message of "Sat, 06 Sep 2014 23:20:35 +0200")

René Scharfe <l.s.r@web.de> writes:

> Am 06.09.2014 um 21:20 schrieb David Aguilar:
>> Add dependent headers so that including a header does not
>> require including additional headers.
>>
>> This makes it so that "gcc -c $header" succeeds for each header.
>>
>> Signed-off-by: David Aguilar <davvid@gmail.com>
>> ---
>
>> diff --git a/branch.h b/branch.h
>> index 64173ab..a61fd1a 100644
>> --- a/branch.h
>> +++ b/branch.h
>> @@ -3,6 +3,9 @@
>>
>>   /* Functions for acting on the information about branches. */
>>
>> +#include "cache.h"
>> +#include "strbuf.h"
>
> cache.h includes strbuf.h, so the line above isn't necessary.

I think the primary objective of these two patches is to allow
anybody who wants to use a feature X to include the header H that is
about that feature without having to know what other header F and G
are required by the implementation of header H.

And I think that cuts both ways.  Somebody who wants to use a
feature defined in the header H shouldn't have to know that other
headers I and J he includes to use other features from happen to
already include H for their own use.

Here, branch.h does want to see "struct strbuf" for its own use, so
including "strbuf.h" is the right thing to do and in line with the
primary objective, no?  It shouldn't have to know that "cache.h"
happens to include "strbuf.h".

While assessing these two patches, we would need to decide what to
do with the "include git-compat-util.h first in any C source" rule
in the coding guidelines.  I think it is still a good practical
exception (in the sense that even if you do not use FLEX_ARRAY,
signed_add_overflows(), etc. yourself, you should follow the rule),
even if we choose to adopt "One that wants to use features from a
header only needs to include that header".

  parent reply	other threads:[~2014-09-08 18:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-06 19:20 [RFC PATCH 1/2] Makefile: add check-headers target David Aguilar
2014-09-06 19:20 ` [RFC PATCH 2/2] headers: include dependent headers David Aguilar
2014-09-06 21:20   ` René Scharfe
2014-09-08 17:50     ` Junio C Hamano
2014-09-09 19:44       ` René Scharfe
2014-09-08 18:05     ` Junio C Hamano [this message]
2014-09-06 21:20 ` [RFC PATCH 1/2] Makefile: add check-headers target René Scharfe
2014-09-06 22:57   ` David Aguilar
2014-09-06 23:58     ` Jeff King
2014-09-07  0:24       ` David Aguilar

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqbnqqng1t.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).