On Fri, Apr 11, 2025 at 2:27 AM Patrick Steinhardt <ps@xxxxxx> wrote: > > On Wed, Apr 09, 2025 at 07:36:47AM -0700, Elijah Newren wrote: > > On Tue, Apr 8, 2025 at 3:37 AM Patrick Steinhardt <ps@xxxxxx> wrote: > > > > > > The `safe_create_leading_directories()` function and its relatives > > > > How is mkdir_in_gitdir() a relative of safe_create_leading_directories()? > > > > I assumed the relation was "called by", but there is no such > > relationship. The rest of the patch looked fine, but I was puzzled > > for a while trying to figure out what this relationship is. > > It's more of a sibling than a child/parent in this case, true. I still > think it makes sense to move it around as it is rather generic in the > functionality it provides and doesn't have anything to do with objects. > > Patrick I fully agree it makes sense to move it and that dir.c is a good place for it, I just think it also makes sense to fix the commit message to avoid the misleading/confusing text by calling out mkdir_in_gitdir() separately since it isn't related to safe_create_leading_directories(). For example, highlighting the text I added between asterisks, you could make it read: The `safe_create_leading_directories()` function and its relatives*, as well as mkdir_in_gitdir()*, are located in "object-file.c", which is not a good fit as they provide generic functionality not related to objects at all. Move them into "dir.c". However, this is a nitpick and probably not worth another re-roll; especially since everything else in your v2 looks great to me.
