Ronnie Sahlberg wrote:
> Update ref_transaction_update() do some basic error checking and return
> true on error. Update all callers to check ref_transaction_update() for error.
> There are currently no conditions in _update that will return error but there
> will be in the future.
> Signed-off-by: Ronnie Sahlberg <sahlb...@google.com>
> builtin/update-ref.c | 10 ++++++----
> refs.c | 9 +++++++--
> refs.h | 10 +++++-----
> 3 files changed, 18 insertions(+), 11 deletions(-)
Revisiting comments from :
* When I call ref_transaction_update, what does it mean that I get
a nonzero return value? Does it mean the _update failed and had
no effect? What will I want to do next: should I try again or
print an error and exit?
Ideally I should be able to answer these questions by reading
the signature of ref_transaction_update and the comment documenting
it. The comment doesn't say anything about what errors
* the error message change for the have_old && !old_sha1 case (to add
"BUG:" so users know the impossible has happened and translators
know not to bother with it) seems to have snuck ahead into patch 28
(refs.c: add transaction.status and track OPEN/CLOSED/ERROR).
* It would be easier to make sense of the error path (does the error
message have enough information? Will the user be bewildered?)
if there were an example of how ref_transaction_update can fail.
There still doesn't seem to be one by the end of the series.
The general idea still seems sensible.
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