Re: [PATCH v3 2/4] remote: use remote_state parameter internally

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 27, 2021, 02:28 UTC
Message-ID: <xmqq8ryfxkuj.fsf@gitster.g>
In-Reply-To: <kl6l7ddz7m54.fsf@chooglen-macbookpro.roam.corp.google.com>

Glen Choo <chooglen@google.com> writes:

Show 16 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>>> c) Replace the backpointer with a remote_state parameter. Expressive and
>>>    fits the paradigm of "defaulting to the repo when needed", but
>>>    interfaces are repetitive and shifts the responsibility of
>>>    correctness to the caller (see v2).
>> 
>> ... if we want to support the what-if callers, I
>> think the best approach would be a slight variant of c) above.
>> 
>> That is, pass branch and remote_state as two parameters, and when
>> branch is not NULL, barf if it is not among remote_state.branches[],
>> to protect against nonsense combinations.
>
> Sounds reasonable to me. The resulting interface would look like the v2
> one, but internally, this additional safety check will prevent misuse.

Hopefully. Of course I think the implementation of the safety would actually be done, not by iterationg over branches[] array, but just checking branch->remote_state == remote_state pointer equality.

Show 5 quoted lines

> This "longer term direction" sounds like what I envisioned with (e). I
> agree that detached HEAD is a state that should be expressed with more
> than just NULL, though I'm not sure that "struct branch" is the correct
> abstraction. No point bikeshedding now of course, we'll cross that
> bridge when we get there ;)

I actually was hoping that the time to cross the bridge was now, though ;-)