From: Michael Haggerty Date: Wed, 02 Apr 2014 10:13:25 GMT Subject: Re: [PATCH v2 26/27] struct ref_update: Add type field Message-ID: <533BE2C5.7050700@alum.mit.edu> In-Reply-To: On 04/01/2014 10:03 PM, Junio C Hamano wrote: > Michael Haggerty writes: > >> This is temporary space for ref_transaction_commit(). >> >> Signed-off-by: Michael Haggerty >> --- > > 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/