Re: [PATCH v2 8/8] check-whitespace: detect if no base_commit is provided

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 12, 2024, 15:12 UTC
Message-ID: <xmqq34oeprui.fsf@gitster.g>
In-Reply-To: <CAOLa=ZSR=mMMnYNrpKc9ivOx4vbzJF7c5e3632NpX4XYFdLMmg@mail.gmail.com>

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

Show 13 quoted lines

>> 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.