Subject: Re: [PATCH v5 2/6] set-head: add new variable for readability

From: Junio C Hamano <[email-masked]>
Date: Wed, 09 Oct 2024 12:26:25 -0700

Bence Ferdinandy <[email-masked]> writes:

> Instead of calling get_main_ref_store(the_repository) multiple times,
> call it once and store in a new refs variable. Although this change
> probably offers some performance benefits, the main purpose is to
> shorten the line lengths of function calls using this variable for
> better readability.
> ---
>
> Notes:
>     v5: new patch (split from the next patch as a preparatory step)

It is a good idea to help making it obvious that we are accessing
the same ref store.

The patch needs to be signed-off, though.

>  builtin/remote.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/remote.c b/builtin/remote.c
> index d8ff440027..353ffd2c43 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -1404,6 +1404,7 @@ static int set_head(int argc, const char **argv, const char *prefix)
>  	int i, opt_a = 0, opt_d = 0, result = 0;
>  	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
>  	char *head_name = NULL;
> +	struct ref_store *refs = get_main_ref_store(the_repository);
>  
>  	struct option options[] = {
>  		OPT_BOOL('a', "auto", &opt_a,
> @@ -1434,7 +1435,7 @@ static int set_head(int argc, const char **argv, const char *prefix)
>  			head_name = xstrdup(states.heads.items[0].string);
>  		free_remote_ref_states(&states);
>  	} else if (opt_d && !opt_a && argc == 1) {
> -		if (refs_delete_ref(get_main_ref_store(the_repository), NULL, buf.buf, NULL, REF_NO_DEREF))
> +		if (refs_delete_ref(refs, NULL, buf.buf, NULL, REF_NO_DEREF))
>  			result |= error(_("Could not delete %s"), buf.buf);
>  	} else
>  		usage_with_options(builtin_remote_sethead_usage, options);
> @@ -1442,9 +1443,9 @@ static int set_head(int argc, const char **argv, const char *prefix)
>  	if (head_name) {
>  		strbuf_addf(&buf2, "refs/remotes/%s/%s", argv[0], head_name);
>  		/* make sure it's valid */
> -		if (!refs_ref_exists(get_main_ref_store(the_repository), buf2.buf))
> +		if (!refs_ref_exists(refs, buf2.buf))
>  			result |= error(_("Not a valid ref: %s"), buf2.buf);
> -		else if (refs_update_symref(get_main_ref_store(the_repository), buf.buf, buf2.buf, "remote set-head", NULL))
> +		else if (refs_update_symref(refs, buf.buf, buf2.buf, "remote set-head", NULL))
>  			result |= error(_("Could not setup %s"), buf.buf);
>  		else if (opt_a)
>  			printf("%s/HEAD set to %s\n", argv[0], head_name);