Christian Couder <christian.couder@xxxxxxxxx> writes:
> 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?
>
The issue is the that this changes existing behavior, since
`ref_transaction_update()` can also be called with the `REF_LOG_ONLY`
flag set.
But, I think it is a worthwhile change, because earlier even for reflog
updates we would show 'refusing to update ref ...' as the message. I'll
add this in and also make a note in the commit message.
Thanks
>> + 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;
>> }
Attachment:
signature.asc
Description: PGP signature
