On Fri, Dec 13, 2024 at 11:36:50AM +0100, Karthik Nayak wrote:
> 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.
> @@ -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
