Ronnie Sahlberg wrote:

> Call ref_transaction_commit with QUIET_ON_ERR and use the error string
> that is returned to print a better log message if/after the transaction
> fails.

Ah, so that's how the transition to a better API happens.  Makes sense.

(A mention of QUIET_ON_ERR in the patch that introduces the &err
parameter could help, or feel free to ignore these comments, since it's
all well by the end of the series.)

> Update the tests to reflect that the log message is now slightly different
>   fatal: update_ref failed: Cannot lock the ref 'some ref'
> versus from the previous
>   fatal: Cannot lock the ref 'some ref'

Makes sense as a demo of what the new code allows, but is this error
message better?  The use of 'git update-ref' is an implementation
detail that the user doesn't need to know about.

I think I'd prefer the result of plain die("%s", err), even though
that's a no-op.

> +++ b/builtin/update-ref.c
> @@ -359,17 +360,18 @@ int cmd_update_ref(int argc, const char **argv, const 
> char *prefix)
>               die("Refusing to perform update with empty message.");
>       if (read_stdin) {
> -             int ret;
>               transaction = ref_transaction_begin();
> -
> +             if (!transaction)
> +                     die("failed to update refs");

This can't happen (xcalloc is defined to die() on malloc failure).
If you want to protect against it anyway, die("BUG: ...")?

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to