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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ronnie Sahlberg <sahlberg@xxxxxxxxxx> 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.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]