Re: [PATCH v2 6/8] refs: introduce the `ref_transaction_update_reflog` function

[Christian Couder <christian.couder@xxxxxxxxx>]

On Fri, Dec 13, 2024 at 11:36 AM Karthik Nayak <karthik.188@xxxxxxxxx> wrote:

> +int ref_transaction_update_reflog(struct ref_transaction *transaction,
> +                                 const char *refname,
> +                                 const struct object_id *new_oid,
> +                                 const struct object_id *old_oid,
> +                                 const char *committer_info, unsigned int flags,
> +                                 const char *msg, unsigned int index,
> +                                 struct strbuf *err)
> +{
> +       struct ref_update *update;
> +
> +       assert(err);
> +
> +       if (!transaction_refname_valid(refname, new_oid, flags, 1, err))
> +               return -1;
> +
> +       flags |= REF_LOG_ONLY | REF_NO_DEREF;

If we could switch the above lines like this:

      flags |= REF_LOG_ONLY | REF_NO_DEREF;

      if (!transaction_refname_valid(refname, new_oid, flags, 1, err))
               return -1;

maybe we wouldn't need transaction_refname_valid() to take an
'unsigned int reflog' argument and we could instead use 'flags &
REF_LOG_ONLY' inside that function?

> +       update = ref_transaction_add_update(transaction, refname, flags,
> +                                           new_oid, old_oid, NULL, NULL,
> +                                           committer_info, msg);
> +       /*
> +        * While we do set the old_oid value, we unset the flag to skip
> +        * old_oid verification which only makes sense for refs.
> +        */
> +       update->flags &= ~REF_HAVE_OLD;
> +       update->index = index;
> +
>         return 0;
>  }