From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 13 Jul 2024 15:47:56 GMT
Subject: Re: [PATCH v3 6/8] ci: run style check on GitHub and GitLab
Message-ID: <xmqqwmlpb8er.fsf@gitster.g>
In-Reply-To: <20240713134518.773053-7-karthik.188@gmail.com>

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

> +  # Since $CI_MERGE_REQUEST_TARGET_BRANCH_SHA is only defined for merged
> +  # pipelines, we fallback to $CI_MERGE_REQUEST_DIFF_BASE_SHA, which should
> +  # be defined in all pipelines.
> +  script:
> +    - |
> +      if test -n "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA"
> +      then
> +        ./ci/run-style-check.sh "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA"
> +      elif test -n "$CI_MERGE_REQUEST_DIFF_BASE_SHA"
> +      then
> +        ./ci/run-style-check.sh "$CI_MERGE_REQUEST_DIFF_BASE_SHA"
> +      else
> +        echo "CI_MERGE_REQUEST_DIFF_BASE_SHA should always exist!"; exit 1
> +      fi

This is fine but we may want to reduce the repetition of the long path
to the script, e.g., by doing something like

	if test -n "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA"
	then
		R=$CI_MERGE_REQUEST_TARGET_BRANCH_SHA
	elif test -n "$CI_MERGE_REQUEST_DIFF_BASE_SHA"
	then
		R="$CI_MERGE_REQUEST_DIFF_BASE_SHA"
	fi

	if test -z "$R"
	then
		echo "CI_MERGE_REQUEST_DIFF_BASE_SHA should always exist!"
		exit 1
	fi
	./ci/run-style-check.sh "$R"

in a separate "after the dust settles" clean-up #leftoverbits topic.
We could replace the first 7 lines with a single-liner

	R=${CI_MERGE_REQUEST_TARGET_BRANCH_SHA-${CI_MERGE_REQUEST_DIFF_BASE_SHA?}}

if we wanted to, but all of that will be mere clean-up changes.