All of lore.kernel.org
 help / color / mirror / Atom feed
From: Derrick Stolee <derrickstolee@github.com>
To: Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: Emily Shaffer <nasamuffin@google.com>, Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH 10/16] object.h: stop depending on cache.h; make cache.h depend on object.h
Date: Thu, 23 Feb 2023 09:17:11 -0500	[thread overview]
Message-ID: <e2948915-8505-2657-7283-e845804dde0d@github.com> (raw)
In-Reply-To: <1357f18db5601cf4fc68a56a2e846d2aed6ad6b6.1677139522.git.gitgitgadget@gmail.com>

On 2/23/2023 3:05 AM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> Things should be able to depend on object.h without pulling in all of
> cache.h.  Move an enum to allow this.
> 
> Note that a couple files previously depended on things brought in
> through cache.h indirectly (revision.h -> commit.h -> object.h ->
> cache.h).  As such, this change requires making existing dependencies
> more explicit in two files.

> diff --git a/diff.h b/diff.h
> index 41eb2c3d428..b90036f5294 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -8,6 +8,7 @@
>  #include "pathspec.h"
>  #include "object.h"
>  #include "oidset.h"
> +#include "strbuf.h"
>  
>  /**
>   * The diff API is for programs that compare two sets of files (e.g. two trees,
> @@ -71,7 +72,6 @@ struct oid_array;
>  struct option;
>  struct repository;
>  struct rev_info;
> -struct strbuf;
>  struct userdiff_driver;

This inclusion of strbuf.h in diff.h seems like a step in the
opposite direction. What caused you to need to do this?

Looking ahead at the patch titles, I see you will revisit diff.h
in the final patch, so I'll try to see if there are effects there.

> diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
> index 1fe393f4473..ef03b45132e 100644
> --- a/list-objects-filter-options.h
> +++ b/list-objects-filter-options.h
> @@ -1,9 +1,10 @@
>  #ifndef LIST_OBJECTS_FILTER_OPTIONS_H
>  #define LIST_OBJECTS_FILTER_OPTIONS_H
>  
> -#include "cache.h"
> +#include "object.h"
>  #include "parse-options.h"
>  #include "string-list.h"
> +#include "strbuf.h"

Here's another case of including strbuf.h instead of declaring
'struct strbuf'. However, it makes a bit more sense because you
are deleting the cache.h include in this change. It would still
be nice if we didn't need to do it.
 
Thanks,
-Stolee

  reply	other threads:[~2023-02-23 14:18 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-23  8:05 [PATCH 00/16] Header cleanups Elijah Newren via GitGitGadget
2023-02-23  8:05 ` [PATCH 01/16] treewide: ensure one of the appropriate headers is sourced first Elijah Newren via GitGitGadget
2023-02-23  8:05 ` [PATCH 02/16] treewide: remove unnecessary git-compat-util.h includes in headers Elijah Newren via GitGitGadget
2023-02-23 13:52   ` Derrick Stolee
2023-02-23 18:13     ` Elijah Newren
2023-02-23 19:35   ` Junio C Hamano
2023-02-23 19:53     ` Elijah Newren
2023-02-23 22:07       ` Junio C Hamano
2023-02-23 23:41         ` Elijah Newren
2023-02-23  8:05 ` [PATCH 03/16] treewide: remove unnecessary cache.h includes Elijah Newren via GitGitGadget
2023-02-23  8:05 ` [PATCH 04/16] treewide: remove unnecessary cache.h includes in source files Elijah Newren via GitGitGadget
2023-02-23 13:57   ` Derrick Stolee
2023-02-23  8:05 ` [PATCH 05/16] alloc.h: move ALLOC_GROW() functions from cache.h Elijah Newren via GitGitGadget
2023-02-23 14:06   ` Derrick Stolee
2023-02-23 18:22     ` Elijah Newren
2023-02-23 19:40     ` Junio C Hamano
2023-02-23 19:39   ` Junio C Hamano
2023-02-23  8:05 ` [PATCH 06/16] hash.h: move some oid-related declarations " Elijah Newren via GitGitGadget
2023-02-23 14:08   ` Derrick Stolee
2023-02-23 18:25     ` Elijah Newren
2023-02-24 23:55   ` Jonathan Tan
2023-02-23  8:05 ` [PATCH 07/16] hex.h: move some hex-related " Elijah Newren via GitGitGadget
2023-02-23 14:11   ` Derrick Stolee
2023-02-23 19:31     ` Elijah Newren
2023-02-23  8:05 ` [PATCH 08/16] pretty.h: move has_non_ascii() declaration from commit.h Elijah Newren via GitGitGadget
2023-02-23  8:05 ` [PATCH 09/16] ident.h: move ident-related declarations out of cache.h Elijah Newren via GitGitGadget
2023-02-23  8:05 ` [PATCH 10/16] object.h: stop depending on cache.h; make cache.h depend on object.h Elijah Newren via GitGitGadget
2023-02-23 14:17   ` Derrick Stolee [this message]
2023-02-23 19:55     ` Elijah Newren
2023-02-23  8:05 ` [PATCH 11/16] dir.h: refactor to no longer need to include cache.h Elijah Newren via GitGitGadget
2023-02-23  8:05 ` [PATCH 12/16] object-store.h: move struct object_info from cache.h Elijah Newren via GitGitGadget
2023-02-23 14:20   ` Derrick Stolee
2023-02-23 19:55     ` Elijah Newren
2023-02-23  8:05 ` [PATCH 13/16] replace-object.h: move read_replace_refs declaration from cache.h to here Elijah Newren via GitGitGadget
2023-02-23  8:05 ` [PATCH 14/16] treewide: replace cache.h with more direct headers, where possible Elijah Newren via GitGitGadget
2023-02-23  8:05 ` [PATCH 15/16] Remove unnecessary includes of builtin.h Elijah Newren via GitGitGadget
2023-02-23  8:05 ` [PATCH 16/16] diff.h: remove unnecessary include of object.h Elijah Newren via GitGitGadget
2023-02-23 14:27 ` [PATCH 00/16] Header cleanups Derrick Stolee
2023-02-24  0:09 ` [PATCH v2 00/17] " Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 01/17] treewide: ensure one of the appropriate headers is sourced first Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 02/17] treewide: remove unnecessary git-compat-util.h includes in headers Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 03/17] treewide: remove unnecessary cache.h includes Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 04/17] treewide: remove unnecessary cache.h includes in source files Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 05/17] alloc.h: move ALLOC_GROW() functions from cache.h Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 06/17] hash.h: move some oid-related declarations " Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 07/17] hex.h: move some hex-related " Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 08/17] cache.h: remove dependence on hex.h; make other files include it explicitly Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 09/17] pretty.h: move has_non_ascii() declaration from commit.h Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 10/17] ident.h: move ident-related declarations out of cache.h Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 11/17] object.h: stop depending on cache.h; make cache.h depend on object.h Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 12/17] dir.h: refactor to no longer need to include cache.h Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 13/17] object-store.h: move struct object_info from cache.h Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 14/17] replace-object.h: move read_replace_refs declaration from cache.h to here Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 15/17] treewide: replace cache.h with more direct headers, where possible Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 16/17] Remove unnecessary includes of builtin.h Elijah Newren via GitGitGadget
2023-02-24  0:09   ` [PATCH v2 17/17] diff.h: remove unnecessary include of object.h Elijah Newren via GitGitGadget
2023-02-24 14:03   ` [PATCH v2 00/17] Header cleanups Johannes Schindelin
2023-02-25  0:10   ` Jonathan Tan
2023-02-25  0:57     ` Elijah Newren
2023-02-27 13:51   ` Derrick Stolee

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=e2948915-8505-2657-7283-e845804dde0d@github.com \
    --to=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=nasamuffin@google.com \
    --cc=newren@gmail.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.