Re: [PATCH 3/5] refs.c: drop dead code checking lock status in `delete_pseudoref()`

From: Martin Ågren <martin.agren@gmail.com>
Date: May 9, 2018, 04:23 UTC
Message-ID: <CAN0heSps9reV82=Sy0J7g3Ewr2fqiPcCT2XBVPNk2yPi+4+mrQ@mail.gmail.com>
In-Reply-To: <20180508181034.GB7210@sigill.intra.peff.net>

On 8 May 2018 at 20:10, Jeff King <peff@peff.net> wrote:

Show 10 quoted lines

> On Sun, May 06, 2018 at 04:10:29PM +0200, Martin Ågren wrote:
>> Unlike in the previous patch, this function is not prepared for
>> indicating errors via a `strbuf err`, so let's just drop the dead code.
>> Any improved error-handling can be added later.
>
> I suspect this ought to eventually be converted to return an error, to
> match the rest of the refs code. And in that sense, this is a step
> backwards versus leaving the current die_errno in place and dropping the
> LOCK_DIE_ON_ERROR flag. But it probably doesn't matter much either way,
> and whoever does the conversion later can deal with it.

Thank you for your comments on this series. It's much appreciated. I will be rerolling this series with extended commit messages about the lock-handling. Looking over this code again, I notice that a larger cleanup of the error-handling might be warranted. It would indeed feel better to do a minor part of that, instead of taking a small step in the wrong direction...

Martin