On Mon, Apr 27, 2020 at 08:13:28PM +0000, Han-Wen Nienhuys via GitGitGadget wrote:
>
>
> This happens implicitly in the files/packed ref backend; making it
> explicit simplifies adding alternate ref storage backends, such as
> reftable.
As an outsider to this part of the codebase, a little more explanation
could be handy in the commit message. I found the backends you mentioned
in refs, and it seems like they're the only two, but it's not obvious
how this delta is related to those backends. Furthermore, grepping looks
like this function whose behavior is changing is being called from
somewhere else, with no change to that function (and it looks like the
callsite's callback doesn't check whether a ref begins with refs/ or
not).
All this to say - it's hard to convince myself this is a safe change,
and I'd really like to read more to understand why you made it.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
> ---
> refs.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index b8759116cd0..05e05579408 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1569,7 +1569,7 @@ static int do_for_each_ref(struct ref_store *refs, const char *prefix,
>
> int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
> {
> - return do_for_each_ref(refs, "", fn, 0, 0, cb_data);
> + return do_for_each_ref(refs, "refs/", fn, 0, 0, cb_data);
> }
>
> int for_each_ref(each_ref_fn fn, void *cb_data)
> @@ -1629,8 +1629,8 @@ int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
>
> int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
> {
> - return do_for_each_ref(refs, "", fn, 0,
> - DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
> + return do_for_each_ref(refs, "refs/", fn, 0, DO_FOR_EACH_INCLUDE_BROKEN,
> + cb_data);
> }
>
> int for_each_rawref(each_ref_fn fn, void *cb_data)
> --
> gitgitgadget
>
