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: ...")?
Thanks,
Jonathan
--
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