Re: [PATCH 0/5] refs: remove functions without ref store

From: Taylor Blau <me@ttaylorr.com>
Date: May 3, 2024, 18:58 UTC
Message-ID: <ZjUzyNnBW1BAtEc/@nand.local>
In-Reply-To: <20240503173553.GC3631237@coredump.intra.peff.net>

On Fri, May 03, 2024 at 01:35:53PM -0400, Jeff King wrote:

Show 13 quoted lines

> Though maybe an even more radical proposal: now that read_ref_full(),
> etc, are gone, and we have only refs_read_ref_full(), could/should we
> shorten the latter to drop the "refs_" prefix? Its original purpose of
> distinguishing the "takes a ref store vs using the_repository" variants
> is now done, and shorter names are less annoying. But:
>
>   - maybe there is value in having ref-related functions namespaced? We
>     certainly don't cover all ref functions here, though, and aside from
>     tight OO-ish APIs (e.g. strbuf) we don't usually do so at all.
>
>   - the error message for in-flight callers of the "old" names will be
>     even more confusing (it will not be "foo() does not exist" but
>     rather "you did not pass enough arguments to foo()").

I actually thought something like the approach we take in banned.h might be helpful, e.g.:

    #define REFS_DEPRECATE(func) use_refs_##func##_instead

    #define read_ref(refname, oid) REFS_DEPRECATE(read_ref)

Then, we could add a bunch of these to the top of refs.h, which would give semi-helpful compiler messages to those whose series are affected by this change.

But TBH I think this is probably overkill and anybody who encounters an issue like this likely does not need the extra hand.

Thanks, Taylor