Re: [PATCH 1/4] refs: add referent parameter to refs_resolve_ref_unsafe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 06, 2024 at 11:23:18AM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
> 
> > "John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> >
> >>  29 files changed, 64 insertions(+), 52 deletions(-)
> >
> > Wow, the blast radius of this thing is rather big.  Among these
> > existing callers of refs_resolve_ref_unsafe(), how many of them
> > will benefit from being able to pass a non NULL parameter at the end
> > of the series, and more importantly, in the future to take advantage
> > of the new feature possibly with a separate series?
> > ...
> > That way, you do not have to touch those existing callers that will
> > never benefit from the new capability in the future.  You won't risk
> > conflicting with in flight topics semantically, either.
> 
> The same comment applies to [3/4], but I do not want to fix the Cc: header
> again, so I am replying to this message.

I wonder whether we can future-proof the code a bit more by introducing
a struct that contains everything we want to pass to the callback
function. That wouldn't fix the blast radius of this patch, but would
mean that we can easily add additional fielids to that struct in the
future without having to adapt any callers at all anymore.

Something like:

    struct ref_data {
        const char *refname;
        const struct object_id *oid;
        const char *referent;
        int flags;
    };

This would also allow us to get rid of this awful `peel_iterated_oid()`
function that relies on global state in many places, as we can put the
peeled OID into that structure, as well.

Patrick

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux