Subject: Re: [PATCH v2 1/3] remote: move static variables into per-repository struct

From: Junio C Hamano <[email-masked]>
Date: Thu, 14 Oct 2021 11:33:43 -0700

Glen Choo <[email-masked]> writes:

> By "here", I assume you mean "the restructuring doesn't belong in
> this topic" vs "the restructuring belongs in this topic but not this
> patch".

Hmph, it is unclear which one you assumed ;-), but anyway I do not
think it would help to make such a clean-up as a part of this
series.

Adding a pointer to the container to contained types, if done from
the beginning without disrupting the function signature (i.e. a
function that takes a branch instance can now work on a branch that
was taken from any repository, by simply following branch->repo and
using it as a parameter to repo_foo_blah() calls it would make
instead of existing foo_blah() calls), may help reduce the size of
the series greatly, because you wouldn't have to touch may existing
callers.  That would make a great part of this topic.

But if done as a post clean-up step (i.e. first we sprinkle "struct
repo *" pointer as an additional parameter all over the functions
that take an object that clearly belongs to a repository, and then
later shrink the API to remove the extra pointer parameter by
teaching the contained object which repository it belongs to), it
would probably not a good idea to have it as part of this series.
We'd instead stop at "we add a new parameter to functions to tell
in which repository it now should work" in the topic itself.

The "post clean-up" series would be like "the previous series made
it 'working', now let's clean-up the API" follow-up that is done
separately.  One benefit of going that route would be that it would
allow us notice there are cases where contained object may have to
be used in the context of the container that is different from the
container it belongs to, before locking ourselves out of the
possibility by limiting the API.  I personally do not think we'd
reap that benefit as I do not think of a case where it makes sense
to take a branch or a remote from a repository and use it in the
context of another repository (hence my suggestion to teach objects
the repository they belong to), but if others think it is easier to
proceed, then...

Thanks.