On Wed, May 17, 2017 at 05:01:41PM +0200, Michael Haggerty wrote: > On 05/17/2017 03:12 PM, Jeff King wrote: > > On Wed, May 17, 2017 at 02:05:30PM +0200, Michael Haggerty wrote: > > > >> Just because the files backend can't retain reflogs for deleted > >> references is no reason that they shouldn't be supported by the > >> virtual method interface. Let's add them now before the interface > >> becomes truly polymorphic and increases the work. > >> > >> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx> > >> --- > >> builtin/fetch.c | 2 +- > >> builtin/remote.c | 4 ++-- > >> refs.c | 11 ++++++----- > >> refs.h | 12 +++++++----- > >> refs/files-backend.c | 4 ++-- > >> refs/refs-internal.h | 2 +- > >> t/helper/test-ref-store.c | 3 ++- > >> t/t1405-main-ref-store.sh | 2 +- > >> t/t1406-submodule-ref-store.sh | 2 +- > >> 9 files changed, 23 insertions(+), 19 deletions(-) > > > > Having carried a similar patch in GitHub's fork for many years (because > > we maintain an audit log of all ref updates), I expected this to be > > bigger. But I forgot that we did 755b49ae9 (delete_ref: accept a reflog > > message argument, 2017-02-20) a few months ago, which already hit most > > of the ref-deleting callers. This is just making the plural > > delete_refs() interface match. > > > > I think your reasoning above is sound by itself, but that gives an added > > interface: we are making the delete_ref() and delete_refs() interfaces > > consistent. > > I think you meant s/interface/justification/, in which case I agree with > you and I'll mention it in v2. I also noticed that the parameters are > named inconsistently. I'll fix that too. Oops, I think I meant "incentive". But yes, you managed to decipher my rambling correctly. -Peff