Re: [PATCH v2 07/10] refs: root refs can be symbolic refs

Patrick Steinhardt
2024-05-07T10:32:41Z

[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]

On Fri, May 03, 2024 at 03:49:25PM -0500, Justin Tobler wrote:
> On 24/05/02 10:07AM, Patrick Steinhardt wrote:
> > On Tue, Apr 30, 2024 at 12:09:57PM -0500, Justin Tobler wrote:
>
> > > I'm not quite sure I understand why we are changing the behavior of
> > > `is_headref()` here. Do we no longer want to validate the ref exists if
> > > it is symbolic?
> >
> > The implementation does not conform to the definition of a "HEAD" ref.
> > Even before this patch series, a "HEAD" ref could either be a symbolic
> > or a regular ref. So to answer the question of "Is this a HEAD ref?" you
> > only need to check whether the ref exists, not whether its target
> > exists.
>
> Thanks Patrick! I think this explantion might be good to add to the
> commit message.

I'll restructure this a bit. In fact, we can even get rid of
`is_headref()` completely as it is now covered by `is_root_ref()`, and
there are no callers of `is_headref()`.

> > > In a prior commit, `is_headref()` is commented to mention that we check
> > > whether the reference exists. Maybe that could use some additional
> > > clarification?
> >
> > Which particular commit do you refer to? It's not part of this series,
> > is it?
>
> I'm refering to the comment added above `is_headref()` in
> (refs: classify HEAD as a root ref, 2024-04-30):
>
> "Check whether the reference is "HEAD" and whether it exists."
>
> Maybe I misunderstand its intent though.

Ah, now I get what you're saying. Yeah, this could indeed use a
clarification. I'll add it to `is_root_ref()` though given that
`is_headref()` will go away.

Patrick