Re: [PATCH 00/16] path: remove dependency on `the_repository`

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 7, 2025, 06:16 UTC
Message-ID: <Z6WlLdwvHuBZg_Is@pks.im>
In-Reply-To: <Z6Tf1uSjKpXsg9aO@ArchLinux>

On Fri, Feb 07, 2025 at 12:14:14AM +0800, shejialuo wrote:

Show 20 quoted lines

> On Thu, Feb 06, 2025 at 08:57:56AM +0100, Patrick Steinhardt wrote:
> > Hi,
> > 
> > this patch series removes the dependency on `the_repository` from the
> > "path" subsystem. The series is structured as follows:
> > 
> >   - Patches 1 to 5 unifyf the interfaces that can be used to retrieve
> >     repository paths (gitdir, commondir, workdir and submodule paths)
> >     and adapts callers accodringly.
> > 
> >   - Patches 6 to 12 drop repository path functions that had an implicit
> >     dependency on `the_repository`.
> > 
> >   - Patches 13 to 16 refactor "path.c"-internal code to stop depending
> >     on `the_repository`.
> > 
> 
> Thanks for working on this. However, I feel a little hard when reviewing
> the code for patch 13 to patch 16. They are not so relevant to the
> previous twelfth patches.

Thanks for your review!

> One thing I want to recommend is that we may combine the first and the
> second step. It is a little wired that we first refactor the code then
> we use the new one. When reading the code, I have to step back to the
> previous patch to understand something.

But those are about very different things. The first step fills in missing interfaces and unifies the calling conventions, whereas the second step removes users of functions that don't have a dependency on `the_repository`. From my perspective, mixing these two steps up with one another would only lead to confusion as we're doing too many things at once.

Patrick