From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 12 Jul 2024 15:12:05 GMT
Subject: Re: [PATCH v2 8/8] check-whitespace: detect if no base_commit is provided
Message-ID: <xmqq34oeprui.fsf@gitster.g>
In-Reply-To: <CAOLa=ZSR=mMMnYNrpKc9ivOx4vbzJF7c5e3632NpX4XYFdLMmg@mail.gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

>> Another thing that I find somewhat disturbing is that the
>> conditional seems the other way around.  It shouldn't be saying "If
>> B is not available, use A, otherwise use B", as if A is known to be
>> usable always.  It should be more like ...
>> shouldn't it?
>>
>
> Agreed, that a comment would be nice here. Will add if I reroll!
>
> In this case A ("$CI_MERGE_REQUEST_DIFF_BASE_SHA") is known to be usable
> always [1].
>
> [1]: https://docs.gitlab.com/ee/ci/variables/predefined_variables.html

Ok, if so check the one you want to use, if exists, first, and then
fall back to what is supposed to exist always but is your second
choice, e.g.,

 	if test -n "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA"
 	then
 		ci/check-whitespace.sh "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA"
 	elif 
 	then test -n "$CI_MERGE_REQUEST_DIFF_BASE_SHA"
 		ci/check-whitespace.sh "$CI_MERGE_REQUEST_DIFF_BASE_SHA"
 	else
 		BUG CI_MERGE_REQUEST_DIFF_BASE_SHA should always exist!
 	fi

Thanks.