Re: [PATCH] setup: only die on invalid .git under RUN_SETUP

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 23, 2021, 15:21 UTC
Message-ID: <xmqq8s1xqcnq.fsf@gitster.g>
In-Reply-To: <87tukls7ax.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 16 quoted lines

>>> +	int die_on_error = !nongit_ok;
>>> +	enum discovery_result discovery;
>>> ...    	/*
>>> +	case GIT_DIR_GITFILE_NOT_A_REPO:
>>> +		if (!nongit_ok)
>>> +			die(_("not a git repository: %s"), dir.buf);
>>> +		*nongit_ok = 1;
>>> +		break;
>>> +	case GIT_DIR_INVALID_GITFILE:
>>> +		if (!nongit_ok)
>>
>> Variable die_on_error could be used in two `if`s above.
>
> Re-reading my own code I think it's better just to drop die_on_error
> entirely and use !nongit_ok consistently, as the rest of the function
> does. What do yo think?

I think "not X_ok" means we do not consider X is OK, and agree with you that the code is clearer without an extra indirection (I do not know if you meant to address me, though).