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

> Ronnie Sahlberg wrote:
>
>> Please pull my ref-transactions branch.
>
> I'm at bd5736cb (2014-05-21 13:46) now.
>
>> On Wed, May 21, 2014 at 3:00 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>>> Ronnie Sahlberg wrote:
>
>>>> --- 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?
>
>> 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.