On Fri, May 03, 2024 at 12:20:54PM -0700, Junio C Hamano wrote: > Taylor Blau <me@xxxxxxxxxxxx> writes: > > > I don't think it's wrong per-se to use the_repository here, but it does > > create something to clean up in the future. > > Very true. This change can be mechanically reproduced on top of an > updated codebase. Such a semantic fix should come on top and it is > better to leave them clearly separated. Agreed. As I mentioned in my patch series that gets rid of `the_index`, I'm taking a bottom-up approach to these kinds of refactorings. At every step, I want to move `the_repository` one layer further up the call chain. This of course means that we're now in a state where many of the callers already have a proper repository at hand, but don't use it. Those would then get addressed in the next step. I think leaving that cleanup to a future series needs to be fine. While it punts some work to the future, that is a necessity in any large-scope refactorings like getting rid of `the_repository` anyway. Otherwise, the scope of any such patch series would likely explode. But to me, the main benefit is that we do not have to worry about whether the refactoring is correct or not. We know that it behaves exactly the same as before, which may not be the case when we started to use caller-provided repositories. So I'd rather want to keep mechanical patch series like this one separated from patch series that actually start to change semantics. Patrick
Attachment:
signature.asc
Description: PGP signature
