Subject: Re: [PATCH v3 6/8] ci: run style check on GitHub and GitLab

From: Junio C Hamano <[email-masked]>
Date: Sat, 13 Jul 2024 08:47:56 -0700

Karthik Nayak <[email-masked]> 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.