From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 02 Sep 2025 08:50:33 GMT
Subject: Re: [PATCH v2 10/16] packfile: refactor `install_packed_git()` to work on packfile store
Message-ID: <aLav2QQgO8dRJWNY@pks.im>
In-Reply-To: <aK0X6FF/vX9kujFR@nand.local>

On Mon, Aug 25, 2025 at 10:11:52PM -0400, Taylor Blau wrote:
> On Thu, Aug 21, 2025 at 09:39:08AM +0200, Patrick Steinhardt wrote:
> > The `install_packed_git()` functions adds a packfile to a specific
> > object store. Refactor it to accept a packfile store instead of a
> > repository to clarify its scope.
> 
> All of the refactoring here looks straightforward and correct to me. I
> admit that I have a vague preference towards keeping the word "install"
> in the function name here, since it (to me) suggests that the packfile
> in question is going to be used for lookups, whereas "add" is a bit
> more generic.
> 
> I don't feel strongly about it, though, so if you have a preference
> towards "add" then I'm fine with that.

I think "install" would make sense if there was a mode where you can set
up a packfile via the store that is _inactive_, but that's not the case
anymore. It was before the changes to introduce the packfile store,
where "add" only instantiated the packfile whereas "install" added it to
the store. But that difference is basically going away now, so as soon
as you add a packfile to the store it immediately becomes active.

So I picked "add" because it's a bit shorter compared to "install", and
because from my point of view the previous distinction isn't necessary
anymore.

I'll keep this as-is for now, but same as you: I don't really feel all
that strongly about it.

Patrick