Re: [PATCH v8 30/44] refs.c: add transaction.status and track OPEN/CLOSED/ERROR

On Wed, May 21, 2014 at 3:22 PM, Jonathan Nieder [off-list ref] wrote:

Ronnie Sahlberg wrote:

quoted

Please pull my ref-transactions branch.

I'm at bd5736cb (2014-05-21 13:46) now.

quoted

On Wed, May 21, 2014 at 3:00 PM, Jonathan Nieder [off-list ref] wrote:

quoted

Ronnie Sahlberg wrote:

quoted

quoted

quoted

--- a/refs.c
+++ b/refs.c

@@ -3308,6 +3308,12 @@ struct ref_update {
      const char refname[FLEX_ARRAY];
 };

+enum ref_transaction_status {
+     REF_TRANSACTION_OPEN   = 0,
+     REF_TRANSACTION_CLOSED = 1,
+     REF_TRANSACTION_ERROR  = 2,

What is the difference between _TRANSACTION_CLOSED and
_TRANSACTION_ERROR?

Closed is a transaction that has been committed successfully, and
which we can not do any more updates onto.
Error is a transaction that has failed, and which we can not do any
more updates onto.

That means that both mean the caller cannot do any more updates,
right?  Why not call them both _CLOSED?

quoted

The distinction could be useful if in the future we add support to
reuse a transaction

If the distinction becomes useful in the future, wouldn't that
be the moment to add a new state?

I don't mean that there has to be a big useful distinction.  E.g.,
maybe the idea is that when using a debugger it can be useful to see
the difference between _ERROR and _CLOSED or something, and I think
that would be fine as long as the intended meaning is documented
(e.g., using comments).  My only complaint is that it's hard to
understand the code and keep track of which status should be used in a
given place unless there is some distinction between them.

I have documented the transaction states in refs.c

Thanks.
ronnie sahlberg