Ronnie Sahlberg wrote: > --- a/refs.c > +++ b/refs.c [...] > @@ -2515,24 +2510,18 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err) > > int delete_ref(const char *refname, const unsigned char *sha1, int delopt) > { > - struct ref_lock *lock; > - int ret = 0, flag = 0; > + struct ref_transaction *transaction; > [...] > - lock = lock_ref_sha1_basic(refname, sha1, delopt, &flag); > + if (!transaction || > + ref_transaction_delete(transaction, refname, sha1, delopt, > + sha1 && !is_null_sha1(sha1), &err) || What should happen when is_null_sha1(sha1)? In that case the caller has asked us to check that the ref doesn't exist before deleting it, which doesn't make a lot of sense, but the old code did exactly that if I read it correctly. The new code seems to disable verification instead. Do we know that no callers call delete_ref with such an argument? Would it make sense to just die with a "BUG:" message to make the contract more clear? [...] > - unlink_or_warn(git_path("logs/%s", lock->ref_name)); When does the reflog get deleted in the new code? Thanks, Jonathan -- 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