On Wed, Apr 09, 2025 at 07:42:38AM -0700, Elijah Newren wrote: > On Tue, Apr 8, 2025 at 3:35 AM Patrick Steinhardt <ps@xxxxxx> wrote: > > > > Hi, > > > > "object-file.c" is quite a grab-bag of all kinds of different functions. > > Many of these functions aren't really a good fit though and should be > > owned by a different subsystem. This patch series tries to split up > > concerns a bit better by splitting out this functionality into other > > files: > > > > - `safe_create_leading_directories()` is moved into "dir.c". > > - `xmmap()` is moved into "wrapper.c". > > - `git_open_cloexec()` is moved into "compat/open.c". > > - Several functions attached to `struct index_state` are moved into > > "read-cache.c". > > - Several functions related to `struct object_store` are moved into a > > new file "object-store.c". > > > > "object-file.c" now mostly contains logic to read and write loose object > > files, whereas "object-store.c" contains the higher-level logic to > > manage different object directories for a repository. Eventually, these > > will become the loose object backend as well as the `struct ref_store` > > equivalent for objects, respectively. > > > > The series is built on top of 9d22ac51228 (The third batch, 2025-04-07) > > with ps/object-wo-the-repository at 9442b1c919a (Merge remote-tracking > > branch 'junio/ps/object-wo-the-repository' into HEAD, 2025-04-08) merged > > into it. > > I was able to apply with these instructions, and looking over the > series locally with --color-moved was very helpful to verify that you > were moving functions without modifying them (or, to notice where you > did modify them and why, e.g. removing the static declaration). > > I very much appreciate the grouping of functions by area that is done > here with read-cache.c, wrapper.c, compat/open.c, dir.c, and the new > object-store.c, as well as the new enums to make the purpose of the > HASH_* flags clearer, and the reduction in the number of globals. > > I only had a few comments on a couple of the patch commit messages. Thanks for your review! Patrick
