Web lists-archives.com

Re: [PATCH 07/23] ref_store: take `logmsg` parameter when deleting references




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