All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Emily Shaffer <emilyshaffer@google.com>
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Ramsay Jones <ramsay@ramsayjones.plus.com>
Subject: Re: [PATCH] Makefile: drop GEN_HDRS
Date: Mon, 16 Dec 2019 17:40:57 -0800	[thread overview]
Message-ID: <20191217014057.GE10734@google.com> (raw)
In-Reply-To: <20191217012756.GQ135450@google.com>

Emily Shaffer wrote:
> On Mon, Dec 16, 2019 at 02:20:14PM -0500, Jeff King wrote:

>> But this would enable us to start checking command-list.h. I'm on the
>> fence on whether that's useful or not; the patch below makes it pass,
>> but I'm not sure if that is really turning up any useful problems. I
>> suppose somebody besides help.c could include command-list.h, in which
>> case some of those MAYBE_UNUSED bits could become useful.
>
> Firstly, I think if someone besides help.c includes command-list.h it
> blows up because there's no include guards :)
>
> My gut wants to say, "I need to be sure my generated header is correct!"
> But it seems that will also be checked when I try to include that header
> from something actually important. So maybe it's not actually useful.
> But then it seems like hdr-check target isn't that useful for anybody,
> since those headers should always be included sometime down the road (or
> why have them). So that makes me think I must still be missing
> something, like maybe I parsed hdr-check wrong.

"git log -Shdr-check Makefile" gives a hint:

  $ git log -Shdr-check Makefile
  commit ebb7baf02f69f2164b1f89148945d18c376fc6a8
  Author: Ramsay Jones <ramsay@ramsayjones.plus.com>
  Date:   Wed Sep 19 01:07:08 2018 +0100

      Makefile: add a hdr-check target

      Commit ef3ca95475 ("Add missing includes and forward declarations",
      2018-08-15) resulted from the author employing a manual method to
      create a C file consisting of a pair of pre-processor #include
      lines (for 'git-compat-util.h' and a given toplevel header), and
      fixing any resulting compiler errors or warnings.

      Add a Makefile target to automate this process.

So it's primarily about making sure that each header is #include-able
on its own (i.e., that it #includes the headers for any types it
relies on).

That seems like something I'd want to hold for command-list.h, too. :)

Thanks,
Jonathan

  reply	other threads:[~2019-12-17  1:41 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-13 23:25 [PATCH] Makefile: drop GEN_HDRS Junio C Hamano
2019-12-14  0:38 ` Jeff King
2019-12-14  1:00   ` Jeff King
2019-12-16 18:55     ` Junio C Hamano
2019-12-16 19:20       ` Jeff King
2019-12-17  1:27         ` Emily Shaffer
2019-12-17  1:40           ` Jonathan Nieder [this message]
2019-12-17  5:22           ` Jeff King
2019-12-17  1:43         ` Jonathan Nieder
2019-12-17  5:28           ` Jeff King
2019-12-17 11:35             ` vcxproj target, was " Johannes Schindelin
2019-12-19  4:51               ` Jeff King

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=20191217014057.GE10734@google.com \
    --to=jrnieder@gmail.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=ramsay@ramsayjones.plus.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.