Re: [PATCH v3 2/7] refs: teach arbitrary repo support to iterators

From: Glen Choo <chooglen@google.com>
Date: Oct 7, 2021, 19:31 UTC
Message-ID: <kl6ly274y6l6.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <c8799d408f2a3b4421db14d756540f287e17b3d1.1632956589.git.jonathantanmy@google.com>

Show 7 quoted lines

> @@ -1139,7 +1142,7 @@ static int should_pack_ref(const char *refname,
>  		return 0;
>  
>  	/* Do not pack broken refs: */
> -	if (!ref_resolves_to_object(refname, oid, ref_flags))
> +	if (!ref_resolves_to_object(refname, the_repository, oid, ref_flags))
>  		return 0;

Nit: It would be nice to have a NEEDSWORK the explain why the_repository is
hardcoded instead of the commit message. Without having something in the
code, it is a little surprising to see the_repository when we are so
close to removing the_repository from refs/ altogether.

This in the spirit of your check in patch 3, where we explicitly warn readers when non-the_repository is not supported.

  +	if (iter->repo != the_repository)
  +		BUG("peeling for non-the_repository is not supported");