Thanks.
Comment added and whitespace fixed.
On Tue, May 13, 2014 at 4:10 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> Ronnie Sahlberg wrote:
>
>> Add a strbuf argument to _commit so that we can pass an error string back to
>> the caller. So that we can do error logging from the caller instead of from
>> _commit.
>>
>> Longer term plan is to first convert all callers to use onerr==QUIET_ON_ERR
>> and craft any log messages from the callers themselves and finally remove the
>> onerr argument completely.
>
> Very nice.
>
> [...]
>> +++ b/refs.c
> [...]
>> @@ -3443,6 +3444,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,
>> update->flags,
>> &update->type, onerr);
>> if (!update->lock) {
>> + if (err)
>> + strbuf_addf(err ,"Cannot lock the ref '%s'.",
>> + update->refname);
>
> Tiny nit: whitespace.
>
> [...]
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -268,9 +268,12 @@ void ref_transaction_delete(struct ref_transaction *transaction,
>> * Commit all of the changes that have been queued in transaction, as
>> * atomically as possible. Return a nonzero value if there is a
>> * problem. The ref_transaction is freed by this function.
>> + * If err is non-NULL we will add an error string to it to explain why
>> + * the transaction failed.
>
> Probably worth mentioning the error string doesn't end with a newline
> so the caller knows how to use it.
>
> With the whitespace fix and with or without the comment tweak,
> Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
>
> diff --git i/refs.c w/refs.c
> index 64e8feb..2ca3169 100644
> --- i/refs.c
> +++ w/refs.c
> @@ -3445,7 +3445,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
> &update->type, onerr);
> if (!update->lock) {
> if (err)
> - strbuf_addf(err ,"Cannot lock the ref '%s'.",
> + strbuf_addf(err, "Cannot lock the ref '%s'.",
> update->refname);
> ret = 1;
> goto cleanup;
> diff --git i/refs.h w/refs.h
> index ff87e14..d45212f 100644
> --- i/refs.h
> +++ w/refs.h
> @@ -268,8 +268,8 @@ void ref_transaction_delete(struct ref_transaction *transaction,
> * Commit all of the changes that have been queued in transaction, as
> * atomically as possible. Return a nonzero value if there is a
> * problem. The ref_transaction is freed by this function.
> - * If err is non-NULL we will add an error string to it to explain why
> - * the transaction failed.
> + * If err is non-NULL we will append an error string (with no trailing
> + * newline) to it to explain why the transaction failed.
> */
> int ref_transaction_commit(struct ref_transaction *transaction,
> const char *msg, struct strbuf *err,
--
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