On Tue, Jan 19, 2021 at 06:33:34PM -0500, Jeff King wrote: > On Tue, Jan 19, 2021 at 06:08:31PM -0500, Taylor Blau wrote: > > That said, you could shorten this to use 'for_each_tag_ref()' instead > > (which compiles to the exact same thing). > > You'd end up with "v1.2.3" in the refname field of the callback then, > rather than "refs/tags/v1.2.3". So we'd definitely need to drop the > prefix check in add_ref_tag() then (though I think it is a good idea > anyway). But I'm also not sure if it would interfere with peel_ref(), > and its magic for using packed-refs peeled information. Ack, thanks for correcting me. Looking at ref_peel_ref(), I am almost positive that calling peel_ref() on "v1.2.3" (referring to "refs/tags/v1.2.3") wouldn't work. So I think the current implementation is good. Thanks, Taylor