From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Michael Haggerty <mhagger@alum.mit.edu>,
Philip Oakley <philipoakley@iee.org>,
git@vger.kernel.org
Subject: Re: [PATCH v3 05/14] Change fetch_pack() and friends to take string_list arguments
Date: Mon, 17 Sep 2012 18:17:10 -0400 [thread overview]
Message-ID: <20120917221710.GA555@sigill.intra.peff.net> (raw)
In-Reply-To: <7vd31ks3ls.fsf@alter.siamese.dyndns.org>
On Mon, Sep 17, 2012 at 03:10:07PM -0700, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
> > But how far should this policy be taken? It seems to me that strict
> > adherence to the policy would dictate that *.h files should *never*
> > include other git project files.
>
> I wouldn't call that a "policy". It's something we think about when
> adding a new "#include" to see if it is worth adding and/or if it is
> the right place to add it to reduce code churn.
>
> As I said, what policy to pick and stick to is open to discussion,
> and I wanted to leave it outside the scope of this series. As it
> has been cooking in 'next', I do not think it is worth reverting the
> inclusion of "string-list.h" to delay this topic. It is something
> that can and should be cleaned up when we decide to pick the
> inclusion policy and enforce it. If we choose to go in the other
> direction, we would end up adding it back, so let's keep it as-is
> for now.
I will admit that I usually follow the opposite policy of what you have
suggested, and include dependent headers in the .h files. Mostly just
because it makes things simpler for the user of the header file, and
there aren't really downsides (yes, you can have weird dependency-order
issues, but in practice those are rare, and they generate very obvious
compile-time errors).
What I think would be much more productive is breaking apart gigantic
includes like cache.h into more reasonable modules, which would mean
less frequent recompilation when an uninteresting part of the header
changes. But git is reasonably fast to compile as it is, so I have never
quite decided that it is worth the human effort to go in that direction.
-Peff
next prev parent reply other threads:[~2012-09-17 22:17 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-09 6:19 [PATCH v3 00/14] Clean up how fetch_pack() handles the heads list Michael Haggerty
2012-09-09 6:19 ` [PATCH v3 01/14] t5500: add tests of error output for missing refs Michael Haggerty
2012-09-09 6:19 ` [PATCH v3 02/14] t5500: add tests of fetch-pack --all --depth=N $URL $REF Michael Haggerty
2012-09-10 20:46 ` Junio C Hamano
2012-09-10 21:53 ` Michael Haggerty
2012-09-18 23:37 ` Philip Oakley
2012-09-09 6:19 ` [PATCH v3 03/14] Rename static function fetch_pack() to http_fetch_pack() Michael Haggerty
2012-09-09 6:19 ` [PATCH v3 04/14] fetch_pack(): reindent function decl and defn Michael Haggerty
2012-09-09 6:19 ` [PATCH v3 05/14] Change fetch_pack() and friends to take string_list arguments Michael Haggerty
2012-09-10 20:56 ` Junio C Hamano
2012-09-17 12:24 ` Michael Haggerty
2012-09-17 22:10 ` Junio C Hamano
2012-09-17 22:17 ` Jeff King [this message]
2012-09-18 20:49 ` Junio C Hamano
2012-09-09 6:19 ` [PATCH v3 06/14] filter_refs(): do not check the same sought_pos twice Michael Haggerty
2012-09-09 6:19 ` [PATCH v3 07/14] fetch_pack(): update sought->nr to reflect number of unique entries Michael Haggerty
2012-09-09 6:19 ` [PATCH v3 08/14] filter_refs(): delete matched refs from sought list Michael Haggerty
2012-09-09 6:19 ` [PATCH v3 09/14] filter_refs(): build refs list as we go Michael Haggerty
2012-09-10 21:18 ` Junio C Hamano
2012-09-09 6:19 ` [PATCH v3 10/14] filter_refs(): simplify logic Michael Haggerty
2012-09-09 6:19 ` [PATCH v3 11/14] cmd_fetch_pack(): return early if finish_connect() fails Michael Haggerty
2012-09-09 6:19 ` [PATCH v3 12/14] fetch-pack: report missing refs even if no existing refs were received Michael Haggerty
2012-09-09 6:19 ` [PATCH v3 13/14] cmd_fetch_pack(): simplify computation of return value Michael Haggerty
2012-09-09 6:19 ` [PATCH v3 14/14] fetch-pack: eliminate spurious error messages Michael Haggerty
2012-09-09 10:20 ` [PATCH v3 00/14] Clean up how fetch_pack() handles the heads list Junio C Hamano
2012-09-09 13:05 ` Jeff King
2012-09-09 18:15 ` Junio C Hamano
2012-09-10 21:59 ` Michael Haggerty
2012-09-10 22:10 ` Junio C Hamano
2012-09-17 12:55 ` Michael Haggerty
2012-09-17 20:39 ` Junio C Hamano
2012-09-10 21:21 ` 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=20120917221710.GA555@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mhagger@alum.mit.edu \
--cc=philipoakley@iee.org \
/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.
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 NNTP newsgroup(s).