Re: [PATCH 5/5] ci: add support for GitLab CI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27/10/2023 09:17, Patrick Steinhardt wrote:
+	CI_BRANCH="$CI_COMMIT_REF_NAME"
+	CI_COMMIT="$CI_COMMIT_SHA"

assignments need no quoting to prevent word splitting.
repeats below.

+	case "$CI_JOB_IMAGE" in

... as does the selector in case statements.

True, but I'm simply matching the coding style in this script.

I think it is quite common for us to quote variables when it isn't strictly necessary as it makes it clear to anyone reading the script that there is no word splitting going on and ensures that we don't start splitting the variable if the contents changes in the future.

--- a/ci/print-test-failures.sh
+++ b/ci/print-test-failures.sh
@@ -51,6 +51,12 @@ do
			tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
			continue
			;;
+		gitlab-ci)
+			mkdir -p failed-test-artifacts
+			cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
+			tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"

you're just following the precedent, but imo it's more legible to quote the
entire string, not just the variable expansion. the code doesn't even agree
with itself, as the line directly above apparently agrees with me.

regards

Yeah, as you say, this is another case where I follow precedent. I
honestly don't quite care which way we go in this case.

Yes, if you're following existing practice I don't think this is worth worrying about.

Best Wishes

Phillip





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux