On Tue, Apr 22, 2014 at 12:11 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Ronnie Sahlberg <sahlb...@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.
>
That is a good idea.
I will try to address that in the next respin.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
