Re: [PATCH 02/11] repository: introduce repo_read_index_or_die

[Duy Nguyen <pclouds@gmail.com>]

On Mon, May 21, 2018 at 9:27 PM, Stefan Beller <sbeller@google.com> wrote:
> Hi Brandon,
>
>>> One of the reviewers of the series questioned the overall goal of the
>>> series as we want to move away from _die() in top level code but this
>>> series moves more towards it.
>>
>> I've heard every once in a while that we want to move toward this but I
>> don't believe there is an actual effort along those lines just yet.  For
>> that to be the case we would need a clearly defined error handling
>> methodology (aside from the existing "die"ing behavior), which we don't
>> currently have.
>
> We have the example in the refs code, which I would want to
> imitate. :)
>
> /*
>  * Return 0 if a reference named refname could be created without
>  * conflicting with the name of an existing reference. Otherwise,
>  * return a negative value and write an explanation to err. [...]
>  */
>
> int refs_verify_refname_available(struct ref_store *refs, ...
>     struct strbuf *err);
>
> extern int refs_init_db(struct strbuf *err);

apply.c and sequencer.c too are libified and try hard to avoid die()
if I remember correctly.

> But it is true that there is no active effort currently being pushed.

Yeah. It took a lot of work to put refs code in the current shape
today. I think we just be careful about adding die(). If a function
has no way to return an error, then die() is unavoidable. If it's a
really fatal error, then die() might still be the right choice. But if
not we should try to return an error instead.

Speaking of avoiding die() (and going off-topic again), maybe we
should introduce NO_DIE_PLEASE macro in the same spirit as
NO_THE_INDEX_COMPATIBILITY_MACROS. If the macro is defined, die() and
friends are not declared (which will result in compile errors at least
on DEVELOPER=1 builds). This helps draw boundary between die-able code
and un-die-able code (not perfect, but good enough)
-- 
Duy