From: Jonathan Nieder <jrnieder@gmail.com>
Date: Fri, 12 May 2017 19:06:17 GMT
Subject: Re: [PATCH v5] fetch-pack: always allow fetching of literal SHA1s
Message-ID: <20170512190617.GC27400@aiede.svl.corp.google.com>
In-Reply-To: <CAGf8dgJxz+jA=pA5TRR6vmK=zP=CUenpsBDy2VEtjYB5CO9yuQ@mail.gmail.com>

Jonathan Tan wrote:

> To make the interface less muddy, would you agree with this (untested):
>
> @@ -648,7 +669,9 @@ static void filter_refs(struct fetch_pack_args *args,
>   continue;
> 
>   if ((allow_unadvertised_object_request &
> -    (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
> +     (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1)) ||
> +      (check_tip_oids_initialized(&tip_oids, unmatched, newlist) &&
> oidset_contains(&tip_oids,
> +     &ref->old_oid))) {
>   ref->match_status = REF_MATCHED;
>   *newtail = copy_ref(ref);
>   newtail = &(*newtail)->next;
>
> (making the function-to-abstract be merely an initialization one,
> instead of one that does 2 things). That decreases the scope of the
> function that Jonathan Nieder and Peff wanted, but it might be a
> warranted reduction in scope.

Yeah, that sounds nicer than anything I suggested.

nit that check_ would be clearer as ensure_

Thanks,
Jonathan