On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote: > Change the reference transactions so that we pass the reflog message > through to the create/delete/update function instead of the commit message. > This allows for individual messages for each change in a multi ref > transaction. > > Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx> > Signed-off-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx> > --- > [...] > diff --git a/refs.h b/refs.h > index 3b321c2..f24b2c1 100644 > --- a/refs.h > +++ b/refs.h Would you please document the msg parameter in the block comment that precedes these three declarations? Especially important is the fact that the functions make internal copies of msg, so the caller retains ownership of its copy. You might also mention what happens if msg is NULL (which, as far as I can see, is that a reflog entry is created anyway (except in the case of a delete) but that the entry doesn't contain an explanation). > @@ -297,7 +297,7 @@ int ref_transaction_update(struct ref_transaction *transaction, > const char *refname, > const unsigned char *new_sha1, > const unsigned char *old_sha1, > - int flags, int have_old, > + int flags, int have_old, const char *msg, > struct strbuf *err); > > /* > @@ -312,7 +312,7 @@ int ref_transaction_update(struct ref_transaction *transaction, > int ref_transaction_create(struct ref_transaction *transaction, > const char *refname, > const unsigned char *new_sha1, > - int flags, > + int flags, const char *msg, > struct strbuf *err); It is noteworthy that ref_transaction_delete() accepts a msg parameter, even though we currently delete a reference's entire reflog when the reference is deleted. I prefer to think of this as a shortcoming of the current reference backend, from which future backends hopefully will not suffer. So I like this design choice. However, I think it is worth noting this dichotomy in the commit message and perhaps also in the function documentation. > /* > @@ -326,7 +326,7 @@ int ref_transaction_create(struct ref_transaction *transaction, > int ref_transaction_delete(struct ref_transaction *transaction, > const char *refname, > const unsigned char *old_sha1, > - int flags, int have_old, > + int flags, int have_old, const char *msg, > struct strbuf *err); > > /* > @@ -335,7 +335,7 @@ int ref_transaction_delete(struct ref_transaction *transaction, > * problem. > */ > int ref_transaction_commit(struct ref_transaction *transaction, > - const char *msg, struct strbuf *err); > + struct strbuf *err); > > /* > * Free an existing transaction and all associated data. > [...] Michael -- Michael Haggerty mhagger@xxxxxxxxxxxx http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html