Re: [PATCH v2 02/13] refs.c: use a single exit path from transaction commit and handle onerr

> Ronnie Sahlberg <sahlberg@google.com> writes:
>
>> diff --git a/refs.c b/refs.c
>> index 138ab70..9daf89e 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3414,12 +3414,12 @@ int ref_transaction_commit(struct ref_transaction *transaction,
>>                          const char *msg, enum action_on_err onerr)
>> ...
>> +     if (ret) {
>> +             const char *str = "Cannot commit transaction.";
>> +             switch (onerr) {
>> +             case UPDATE_REFS_MSG_ON_ERR: error(str); break;
>> +             case UPDATE_REFS_DIE_ON_ERR: die(str); break;
>> +             case UPDATE_REFS_QUIET_ON_ERR: break;
>> +             }
>> +     }
>>       return ret;
>>  }
>
> I think this is a response to Michael's earlier review "do different
> callers want to give different error messages more suitable for the
> situation?".  I suspect that the ideal endgame may end up all
> callers passing QUIET_ON_ERR and doing the error message themselves,
> e.g. branch.c::craete_branch() may end something like this:
>
>     ...
>     if (!dont_change_ref)
>         if (ref_transaction_commit(..., UPDATE_REFS_QUIET_ON_ERR))
>                 die_errno(_("Failed to write branch '%s'"),
>                           skip_prefix(ref.buf, "refs/heads/));
>
> And in the meantime until the callers are converted, we may end up
> showing the fallback "Cannot commit transaction." but that would be
> OK during the development and polishing of this series.
>