Re: [PATCH v2 26/27] struct ref_update: Add type field

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Apr 2, 2014, 10:13 UTC
Message-ID: <533BE2C5.7050700@alum.mit.edu>
In-Reply-To: <xmqq8urozuk0.fsf@gitster.dls.corp.google.com>

On 04/01/2014 10:03 PM, Junio C Hamano wrote:

Show 18 quoted lines

> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> This is temporary space for ref_transaction_commit().
>>
>> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
>> ---
> 
> I was about to complain to "*Add* type" that does not say what it is
> used for at all, with "Please do not add something for unknown purpose
> only to utilise it in a later patch".
> 
> But that was before I noticed that these are already used and
> realized that the change is about "moving what is recorded in the
> type array, which is used to receive the existing reftype discovered
> by calling resolve_ref_unsafe() in ref_transaction_commit() and not
> used anywhere else, to a field of individual ref_update structure".
> 
> So it was somewhat of a "Huh?", but perhaps it is OK.

I will expand the comment in v3.

> I wonder if ref-transaction-commit can shrink its parameter list by
> accepting a single pointer to one ref_update?

I don't understand this last point. ref_transaction_commit() has the following signature:

int ref_transaction_commit(struct ref_transaction *transaction,
			   const char *msg, enum action_on_err onerr)

What change are you proposing?

By the way, longer-term, I wonder if msg and maybe action_on_err should be set for each ref_update, rather than for a whole transaction.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/