Re: [PATCH v2 00/14] refs: improvements and fixes for peeling tags

[Toon Claes <toon@iotcl.com>]

Jeff King <peff@peff.net> writes:

>> > I do have one minor complaint, though: the name of that struct. I have a
>> > feeling that the name "struct reference" may cause confusion down the
>> > road because it's so generic, and because "references" and "refs" are so
>> > common in the code. From the names, when would I know when to use
>> > "struct reference" and when "struct ref"?
>> > 
>> > Could we give it a name that ties it to the iteration interface?
>> > Something like iterated_ref, each_ref_data, etc?
>> > 
>> > I know this is minor (and will be annoying to adjust your series), but
>> > I'd rather raise the point now than realize later that it's confusing
>> > and try to change it then.

When I was reviewing these patches, I also had some doubts whether
`reference` was the best name for this struct. Especially because the
comment above says:

    /* A reference passed to `for_each_ref()`-style callbacks. */

But on the other hand, I agree with Patrick:

>> what it contains is as close as you get to a representation of a
>> reference.

So, I'd be fine if this comment was rewritten a little bit, into
something like:

    /* A representation of a reference, for example to be passed to
     * `for_each_ref()`-style callbacks. */

> I am not so much arguing that "struct reference" is misnamed, as that it
> is sufficiently generic that people will reach for it when it is not the
> appropriate tool. It is for passing the ref data to the iterator
> callback, but it probably doesn't make sense in other contexts. Would we
> ever expect anybody to declare their own "struct reference" in a local
> function? I don't think so.

Personally I'd say it's hard to predict something like that. Usually I
tend to lean toward naming that doesn't assume anything that might
happen in the future. And because I think this struct contains what a
reference _is_, I think this name is fine. And whenever we run into the
issue that we need something else to be named `reference`, we revisit
this decision.

> And yes, "struct ref" suffers somewhat from the same problem. It is
> mostly about using refs in one specific space, but the name does not
> really help clarify that. I wouldn't mind seeing that improved, but yes,
> it would be a noisy patch. I don't know if remote_ref is the right name,
> though (the "peer_ref" links mean we store both local and remote refs in
> it, IIRC).





-- 
Cheers,
Toon