On 1/9/2023 12:15 PM, Ævar Arnfjörð Bjarmason wrote: > On Fri, Jan 06 2023, Derrick Stolee wrote: >> On 1/6/2023 5:45 PM, Junio C Hamano wrote: >>> We probably should start paying down such technical debts. We've >>> punted on them too many times, saying "yes this is klunky but what >>> we have is good enough for adding this feature", I suspect? >> >> Yes, I'll make note to prioritize this soon. > > I noted in passing in [1] that I had those patches locally, if I'm > understanding you correctly and you're planning to work on changes > that'll make "istate->repo" always non-NULL. > > I've rebased those on top of your "ds/omit-trailing-hash-in-index". I'm > CI-ing those now & hoping to submit them soon (I've had it working for a > while, but there was some interaction with your patches). Preview at > [2]. > > 1. https://lore.kernel.org/git/221215.865yec3b1j.gmgdl@xxxxxxxxxxxxxxxxxxx/ > 2. https://github.com/avar/git/compare/avar-ds/omit-trailing-hash-in-index...avar/do-not-lazy-populate-istate-repo Thanks for doing this so I don't need to. Some quick pre-submission feedback: your "treewide" patch [1] is far too big and doing too much all at once. It's difficult to know why you're doing things when you're doing them, especially the choices for the validate_cache_repo() calls. In particular, I noticed that you used "the_repository" in some cases where a local "struct repository *" would be better (even if it is currently pointing to the_repository as in builtin/sparse-checkout.c in update_working_directory()). These would be easier to catch in smaller diffs. [1] https://github.com/avar/git/commit/7732a41800dfc8f5dbf909560615d6048d583ed9 Looking forward to your series. -Stolee
