Subject: Re: [PATCH 4/5] cocci: apply rules to rewrite callers of "refs" interfaces

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 6 May 2024 08:35:23 +0200

On Fri, May 03, 2024 at 12:20:54PM -0700, Junio C Hamano wrote:
> Taylor Blau <[email-masked]> 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