Re: [PATCH v2 5/8] refs: add `committer_info` to `ref_transaction_add_update()`

From: Patrick Steinhardt <ps@pks.im>
Date: Dec 13, 2024, 12:24 UTC
Message-ID: <Z1wnbCoidG-lC7ui@pks.im>
In-Reply-To: <20241213-320-git-refs-migrate-reflogs-v2-5-f28312cdb6c0@gmail.com>

On Fri, Dec 13, 2024 at 11:36:50AM +0100, Karthik Nayak wrote:

Show 5 quoted lines

> The `ref_transaction_add_update()` creates the `ref_update` struct. To
> facilitate addition of reflogs in the next commit, the function needs to
> accommodate setting the `committer_info` field in the struct. So modify
> the function to also take `committer_info` as an argument and set it
> accordingly.

I was wondering a bit whether we could instead pull out a `add_update_internal()` function so that we don't need to modify all callers of `ref_transaction_add_update()`. Because ultimately, we don't use the field anywhere except from `ref_transaction_add_reflog_update()` as far as I can see.

This is more of a thought than a strong opinion, so feel free to ignore.

Show 14 quoted lines

> @@ -1190,8 +1191,12 @@ struct ref_update *ref_transaction_add_update(
>  		oidcpy(&update->new_oid, new_oid);
>  	if ((flags & REF_HAVE_OLD) && old_oid)
>  		oidcpy(&update->old_oid, old_oid);
> -	if (!(flags & REF_SKIP_CREATE_REFLOG))
> +	if (!(flags & REF_SKIP_CREATE_REFLOG)) {
> +		if (committer_info)
> +			update->committer_info = xstrdup(committer_info);
> +
>  		update->msg = normalize_reflog_message(msg);
> +	}
>  
>  	return update;
>  }

This can use `xstrdup_or_null()` and then drop the condition.

Patrick