On Mon, Apr 24, 2023 at 11:51 AM Glen Choo <chooglen@xxxxxxxxxx> wrote: > > "Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > > Move the parts of hash.h that do not > > depend upon repository.h into a new file hash-ll.h (the "low level" > > parts of hash.h), and adjust other files to use this new header where > > the convenience inline functions aren't needed. > > Suggestion: To maintain this property, it might be helpful to capture > the rules of hash-ll.h vs hash.h in a top-level comment. > > > diff --git a/hash-ll.h b/hash-ll.h > > new file mode 100644 > > index 00000000000..80509251370 > > --- /dev/null > > +++ b/hash-ll.h > > +const struct object_id *null_oid(void); > > [...] > > +const char *empty_tree_oid_hex(void); > > +const char *empty_blob_oid_hex(void); > > hash-ll.h doesn't depend on repository.h, but these functions' bodies > use the_hash_algo. Does it matter? That's a good point. I think eventually moving these functions from hash-ll.h to hash.h on this basis makes sense. But I kind of view this series (and its predecessors) as focusing on cleaning up header dependencies, with the idea that hopefully we'd go through and do further cleanup of dependencies of C files that would likely result in further changes to some of the headers. I suspect this isn't the only change we'd need to make for that kind of consistency, which might lengthen this already sizable series, and I've probably missed a few similar cases in my previous series as well. > Moving the functions to hash.h requires changing 8 files to #include > "hash.h", all of which seem to be because they were getting hash-ll.h > indirectly via object-name.h. Yeah, seems like a reasonable further change. Are you okay with that being in a future series, or do you think it should be included in this one?