On Mon, May 21, 2018 at 9:27 PM, Stefan Beller <sbeller@xxxxxxxxxx> 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
