From: Junio C Hamano <gitster@pobox•com>
To: David Aguilar <davvid@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [RFC PATCH 3/3] core: improve header dependencies
Date: Tue, 02 Sep 2014 11:32:02 -0700 [thread overview]
Message-ID: <xmqqr3zt27rx.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1409515893-48017-3-git-send-email-davvid@gmail.com> (David Aguilar's message of "Sun, 31 Aug 2014 13:11:33 -0700")
David Aguilar <davvid@gmail•com> writes:
> Remove includes that have already been included by another header.
Hmm, I am not sure if that is a good move, and suspect that it is
incompatible with what your 2/3 attempts to do, at least at the
philosophical level.
I am guessing that your 2/3 wants to see
gcc $header.h
to be happy. One benefit from doing such a change is that sources
that want to use declaration made in $header.h have to include that
$header.h without having to worry about what other things the
implementation detail of $header.h needs. If function F or type T
is declared in header H, you include H and you are done.
That is nice and tidy, but if that is the goal, then after making H
include its own dependency H1 that happen to declare functions F1,
F2 and types T1, T2 (which are necessary for H to be complete as
standalone), if the source that used to include both H and H1
because it uses F and F1 should still explicitly include H1, no?
For example, you dropped "diff.h" from builtin/add.c, but the
implementation of builtin/add.c needs access to diff_options struct,
which is in "diff.h", not whatever happened to include indirectly
that is already included by builtin/add.c. I do not think it is a
good idea, and more importantly I suspect that it is not consistent
with what you tried to do with your 2/3.
But it is entirely possible I am misunderstanding the real
motivation behind these changes. The log message justifies why
removal is safe i.e. "have already been included indirectly", and
the title claims it is an improvement, but there is no explanation
why it is an improvement (which would have also explained the
motivation behind it), so it is a bit hard for me to guess.
next prev parent reply other threads:[~2014-09-02 18:34 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-31 20:11 [RFC PATCH 1/3] stylefix: pointers bind to the variable, not the type David Aguilar
2014-08-31 20:11 ` [RFC PATCH 2/3] headers: improve header dependencies David Aguilar
2014-08-31 20:11 ` [RFC PATCH 3/3] core: " David Aguilar
2014-09-02 18:32 ` Junio C Hamano [this message]
2014-09-02 19:19 ` David Aguilar
2014-09-02 18:19 ` [RFC PATCH 2/3] headers: " Junio C Hamano
2014-09-02 18:33 ` [RFC PATCH 1/3] stylefix: pointers bind to the variable, not the type Junio C Hamano
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=xmqqr3zt27rx.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=davvid@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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