On Thu, Sep 11, 2025 at 07:40:01PM -0400, Taylor Blau wrote: > On Tue, Sep 09, 2025 at 01:02:59PM +0200, Patrick Steinhardt wrote: > > Changes in v4: > > - Small code style improvement as suggested by Junio. > > - Some commit message improvements as suggested by Karthik. > > - Link to v3: https://lore.kernel.org/r/20250902-b4-pks-packfiles-store-v3-0-6925278efeda@xxxxxx > > Thanks for these changes. I think that this series is getting closer, > but I do not think that it is quite ready yet. > > The largest outstanding topic is that this round claims that > get_all_packs() behaves identically to get_packed_git(), but this is not > the case. I mentioned in a response to the patch that removes the latter > function, but I think we should: > > - Permit both get_all_packs() and get_packed_git() to coexist for now. > > - In a follow-up series, transition get_packed_git() callers one-by-one > to use get_all_packs() instead. Each of these commits should include > IMHO a justification that the change is safe as-is, or include fixes > to make it safe. I think any "fixes" here are limited to "if > (p->multi_pack_index) continue;". > > - Once there are no longer any callers of get_packed_git(), we can > remove it. > > If you want to pursue that in this series, I am happy to review and > discuss it, but IMHO there is already enough going on here that I think > it makes more sense to do that in a separate follow-up. Oops, hit "send" too early. The other outstanding topic that I want to raise is that I think transitioning away from get_all_packs() to the new API in all callers is premature. I would *much* rather see us do this call-by-call when it is necessary to do so rather than forcing all callers onto the new API. If there is a compelling reason that we must force callers to all use the new API now, I'm happy to discuss that, but as-is I worry that we are changing things too quickly here. Thanks, Taylor
