On Sat, Apr 22 2023, Elijah Newren via GitGitGadget wrote:

The end-state of this topic is oddly inconsistent in when it uses
includes, and when it uses forward declarations.

As I note in a reply to 20/22 you're adding forward declares there, I
think that's fine, but if you opdet for that why not do that here. The
body of this header only defines one function, which takes a pointer to
a "struct object_id".

Whereas above you did this change:

There we really should include it, as we're not dealing with a pointer
to the "struct object_id", but the struct itself, so we need its
definition, and don't want to find it implicitly.

Then we have this, where we seemingly could avoid the include as well,
and just add a:

	struct git_hash_algo;

Anyway, I'm not saying one is better than the other, I'm just wondering
why you're picking one, but not the other.

Is it because you know that hash-ll.h doesn't bring in other headers, so
its inclusion is OK, whereas later in e.g. 20/22 you avoid including
strbuf.h, because you know that'll bring in string-list.h?

If that's the case maybe we should just move
strbuf_add_separated_string_list() into some "used by merge-ort.c and
merge-recursive.c" file, remove the string-list.h includion from
strbuf.h, and then include "strbuf.h" without fearing the side-effects
elsewhere?