Web lists-archives.com

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




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.

Michael