Re: [PATCH 2/2] refs: switch for_each_replace_ref back to use a ref_store

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jul 31, 2018, 00:18 UTC
Message-ID: <20180731001858.122968-1-jonathantanmy@google.com>
In-Reply-To: <20180730194731.220191-3-sbeller@google.com>

> So let's go back to the clean API, just requiring a ref_store as an
> argument.

Here, you say that we want ref_store as an argument...

Show 8 quoted lines

> -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)
> +int for_each_replace_ref(each_ref_fn fn, void *cb_data)
>  {
> -	return do_for_each_ref(get_main_ref_store(r),
> +	return do_for_each_ref(get_main_ref_store(the_repository),
>  			       git_replace_ref_base, fn,
>  			       strlen(git_replace_ref_base),
>  			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);

...but there is no ref_store as an argument here - instead, the repository argument is deleted with no replacement. I presume you meant to replace it with a ref_store instead? (This will also fix the issue that for_each_replace_ref only works on the_repository.)

Taking a step back, was there anything that prompted these patches? Maybe at least the 2nd one should wait until we have a situation that warrants it (for example, if we want to for_each_replace_ref(), but we only have a ref_store, not a repository).